Skip to content

Commit

Permalink
change tuple indexing
Browse files Browse the repository at this point in the history
this changes the tuple index from `tuple[0]` to `tuple.0` in accordance
with the accepted propsal carbon-language#3646
  • Loading branch information
brymer-meneses committed Aug 3, 2024
1 parent b3fcaf9 commit 8ec0632
Show file tree
Hide file tree
Showing 37 changed files with 339 additions and 476 deletions.
53 changes: 0 additions & 53 deletions toolchain/check/handle_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,6 @@ auto HandleParseNode(Context& /*context*/, Parse::IndexExprStartId /*node_id*/)
return true;
}

// Validates that the index (required to be an IntLiteral) is valid within the
// tuple size. Returns the index on success, or nullptr on failure.
static auto ValidateTupleIndex(Context& context, Parse::NodeId node_id,
SemIR::Inst operand_inst,
SemIR::IntLiteral index_inst, int size)
-> const llvm::APInt* {
const auto& index_val = context.ints().Get(index_inst.int_id);
if (index_val.uge(size)) {
CARBON_DIAGNOSTIC(
TupleIndexOutOfBounds, Error,
"Tuple element index `{0}` is past the end of type `{1}`.", TypedInt,
SemIR::TypeId);
context.emitter().Emit(node_id, TupleIndexOutOfBounds,
{.type = index_inst.type_id, .value = index_val},
operand_inst.type_id());
return nullptr;
}
return &index_val;
}

auto HandleParseNode(Context& context, Parse::IndexExprId node_id) -> bool {
auto index_inst_id = context.node_stack().PopExpr();
auto operand_inst_id = context.node_stack().PopExpr();
Expand Down Expand Up @@ -72,39 +52,6 @@ auto HandleParseNode(Context& context, Parse::IndexExprId node_id) -> bool {
context.node_stack().Push(node_id, elem_id);
return true;
}
case CARBON_KIND(SemIR::TupleType tuple_type): {
SemIR::TypeId element_type_id = SemIR::TypeId::Error;
auto index_node_id = context.insts().GetLocId(index_inst_id);
index_inst_id = ConvertToValueOfType(
context, index_node_id, index_inst_id,
context.GetBuiltinType(SemIR::BuiltinInstKind::IntType));
auto index_const_id = context.constant_values().Get(index_inst_id);
if (index_const_id == SemIR::ConstantId::Error) {
index_inst_id = SemIR::InstId::BuiltinError;
} else if (!index_const_id.is_template()) {
// TODO: Decide what to do if the index is a symbolic constant.
CARBON_DIAGNOSTIC(TupleIndexNotConstant, Error,
"Tuple index must be a constant.");
context.emitter().Emit(node_id, TupleIndexNotConstant);
index_inst_id = SemIR::InstId::BuiltinError;
} else {
auto index_literal = context.insts().GetAs<SemIR::IntLiteral>(
context.constant_values().GetInstId(index_const_id));
auto type_block = context.type_blocks().Get(tuple_type.elements_id);
if (const auto* index_val =
ValidateTupleIndex(context, node_id, operand_inst,
index_literal, type_block.size())) {
element_type_id = type_block[index_val->getZExtValue()];
} else {
index_inst_id = SemIR::InstId::BuiltinError;
}
}
context.AddInstAndPush<SemIR::TupleIndex>(node_id,
{.type_id = element_type_id,
.tuple_id = operand_inst_id,
.index_id = index_inst_id});
return true;
}
default: {
if (operand_type_id != SemIR::TypeId::Error) {
CARBON_DIAGNOSTIC(TypeNotIndexable, Error,
Expand Down
22 changes: 21 additions & 1 deletion toolchain/check/handle_name.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/base/kind_switch.h"
#include "toolchain/check/context.h"
#include "toolchain/check/generic.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/member_access.h"
#include "toolchain/check/name_component.h"
Expand All @@ -22,6 +23,15 @@ auto HandleParseNode(Context& context, Parse::MemberAccessExprId node_id)
auto member_id =
PerformCompoundMemberAccess(context, node_id, base_id, member_expr_id);
context.node_stack().Push(node_id, member_id);
} else if (context.node_stack().PeekIs<Parse::NodeKind::IntLiteral>()) {
auto index_inst_id = context.node_stack().PopExpr();
auto tuple_inst_id = context.node_stack().PopExpr();

auto tuple_value_inst_id =
PerformTupleIndex(context, node_id, tuple_inst_id, index_inst_id);

context.node_stack().Push(node_id, tuple_value_inst_id);

} else {
SemIR::NameId name_id = context.node_stack().PopName();
auto base_id = context.node_stack().PopExpr();
Expand Down Expand Up @@ -52,6 +62,16 @@ auto HandleParseNode(Context& context, Parse::PointerMemberAccessExprId node_id)
auto member_id = PerformCompoundMemberAccess(context, node_id,
deref_base_id, member_expr_id);
context.node_stack().Push(node_id, member_id);
} else if (context.node_stack().PeekIs<Parse::NodeKind::IntLiteral>()) {
auto index_inst_id = context.node_stack().PopExpr();
auto tuple_pointer_inst_id = context.node_stack().PopExpr();
auto tuple_inst_id = PerformPointerDereference(
context, node_id, tuple_pointer_inst_id, diagnose_not_pointer);
auto tuple_value_inst_id =
PerformTupleIndex(context, node_id, tuple_inst_id, index_inst_id);

context.node_stack().Push(node_id, tuple_value_inst_id);

} else {
SemIR::NameId name_id = context.node_stack().PopName();
auto base_id = context.node_stack().PopExpr();
Expand Down
80 changes: 79 additions & 1 deletion toolchain/check/member_access.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "toolchain/check/member_access.h"

#include <optional>

#include "llvm/ADT/STLExtras.h"
#include "toolchain/base/kind_switch.h"
#include "toolchain/check/context.h"
Expand Down Expand Up @@ -391,7 +393,6 @@ auto PerformMemberAccess(Context& context, Parse::NodeId node_id,

return member_id;
}

auto PerformCompoundMemberAccess(Context& context, Parse::NodeId node_id,
SemIR::InstId base_id,
SemIR::InstId member_expr_id)
Expand Down Expand Up @@ -429,4 +430,81 @@ auto PerformCompoundMemberAccess(Context& context, Parse::NodeId node_id,
return member_id;
}

// Validates that the index (required to be an IntLiteral) is valid within the
// tuple size. Returns the index on success, or nullptr on failure.
static auto ValidateTupleIndex(Context& context, Parse::NodeId node_id,
SemIR::Inst operand_inst,
SemIR::IntLiteral index_inst, int size)
-> const llvm::APInt* {
const auto& index_val = context.ints().Get(index_inst.int_id);
if (index_val.uge(size)) {
CARBON_DIAGNOSTIC(
TupleIndexOutOfBounds, Error,
"Tuple element index `{0}` is past the end of type `{1}`.", TypedInt,
SemIR::TypeId);
context.emitter().Emit(node_id, TupleIndexOutOfBounds,
{.type = index_inst.type_id, .value = index_val},
operand_inst.type_id());
return nullptr;
}
return &index_val;
}

auto PerformTupleIndex(Context& context, Parse::NodeId node_id,
SemIR::InstId tuple_inst_id, SemIR::InstId index_inst_id)
-> SemIR::InstId {
tuple_inst_id = ConvertToValueOrRefExpr(context, tuple_inst_id);
auto tuple_inst = context.insts().Get(tuple_inst_id);
auto tuple_type_id = tuple_inst.type_id();

CARBON_KIND_SWITCH(context.types().GetAsInst(tuple_type_id)) {
case CARBON_KIND(SemIR::TupleType tuple_type): {
SemIR::TypeId element_type_id = SemIR::TypeId::Error;
auto index_node_id = context.insts().GetLocId(index_inst_id);
index_inst_id = ConvertToValueOfType(
context, index_node_id, index_inst_id,
context.GetBuiltinType(SemIR::BuiltinInstKind::IntType));
auto index_const_id = context.constant_values().Get(index_inst_id);
if (index_const_id == SemIR::ConstantId::Error) {
index_inst_id = SemIR::InstId::BuiltinError;
} else if (!index_const_id.is_template()) {
// TODO: Decide what to do if the index is a symbolic constant.
CARBON_DIAGNOSTIC(TupleIndexNotConstant, Error,
"Tuple index must be a constant.");
context.emitter().Emit(node_id, TupleIndexNotConstant);
index_inst_id = SemIR::InstId::BuiltinError;

} else {
auto index_literal = context.insts().GetAs<SemIR::IntLiteral>(
context.constant_values().GetInstId(index_const_id));
auto type_block = context.type_blocks().Get(tuple_type.elements_id);
if (const auto* index_val =
ValidateTupleIndex(context, node_id, tuple_inst, index_literal,
type_block.size())) {
element_type_id = type_block[index_val->getZExtValue()];
} else {
index_inst_id = SemIR::InstId::BuiltinError;
}
}

return context.AddInst<SemIR::TupleIndex>(node_id,
{.type_id = element_type_id,
.tuple_id = tuple_inst_id,
.index_id = index_inst_id});
}
default: {
if (tuple_type_id != SemIR::TypeId::Error) {
CARBON_DIAGNOSTIC(
TupleWrongIndexSyntax, Error,
"Type `{0}` does not support array indexing. Did you mean tuple.0?",
SemIR::TypeId);
context.emitter().Emit(node_id, TupleWrongIndexSyntax, tuple_type_id);
}
context.node_stack().Push(node_id, SemIR::InstId::BuiltinError);
}
}

return SemIR::InstId::BuiltinError;
}

} // namespace Carbon::Check
4 changes: 4 additions & 0 deletions toolchain/check/member_access.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ auto PerformCompoundMemberAccess(Context& context, Parse::NodeId node_id,
SemIR::InstId base_id,
SemIR::InstId member_expr_id) -> SemIR::InstId;

auto PerformTupleIndex(Context& context, Parse::NodeId node_id,
SemIR::InstId tuple_inst_id, SemIR::InstId index_inst_id)
-> SemIR::InstId;

} // namespace Carbon::Check

#endif // CARBON_TOOLCHAIN_CHECK_MEMBER_ACCESS_H_
6 changes: 3 additions & 3 deletions toolchain/check/testdata/eval/aggregate.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var tuple_copy: (i32, i32) = (1, 2) as (i32, i32);

var struct_copy: {.a: i32, .b: i32, .c: i32} = {.c = 3, .b = 2, .a = 1} as {.b: i32, .a: i32, .c: i32};

var tuple_index: [i32; 1] = (0,) as [i32; (5, 7, 1, 9)[2]];
var tuple_index: [i32; 1] = (0,) as [i32; (5, 7, 1, 9).2];

var struct_access: [i32; 1] = (0,) as [i32; {.a = 3, .b = 1}.b];

Expand Down Expand Up @@ -174,10 +174,10 @@ var struct_access: [i32; 1] = (0,) as [i32; {.a = 3, .b = 1}.b];
// CHECK:STDOUT: %.loc15_56: i32 = int_literal 2 [template = constants.%.6]
// CHECK:STDOUT: %tuple: %.20 = tuple_value (%.loc15_44, %.loc15_47, %.loc15_50, %.loc15_53) [template = constants.%tuple.2]
// CHECK:STDOUT: %.loc15_54.2: %.20 = converted %.loc15_54.1, %tuple [template = constants.%tuple.2]
// CHECK:STDOUT: %.loc15_57: i32 = tuple_index %.loc15_54.2, %.loc15_56 [template = constants.%.5]
// CHECK:STDOUT: %.loc15_55: i32 = tuple_index %.loc15_54.2, %.loc15_56 [template = constants.%.5]
// CHECK:STDOUT: %.loc15_38.1: type = value_of_initializer %int.make_type_32.loc15 [template = i32]
// CHECK:STDOUT: %.loc15_38.2: type = converted %int.make_type_32.loc15, %.loc15_38.1 [template = i32]
// CHECK:STDOUT: %.loc15_58: type = array_type %.loc15_57, i32 [template = constants.%.13]
// CHECK:STDOUT: %.loc15_57: type = array_type %.loc15_55, i32 [template = constants.%.13]
// CHECK:STDOUT: %.loc15_5: ref %.13 = splice_block file.%tuple_index.var {}
// CHECK:STDOUT: %.loc15_32.2: i32 = int_literal 0 [template = constants.%.15]
// CHECK:STDOUT: %.loc15_32.3: ref i32 = array_index %.loc15_5, %.loc15_32.2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn G() -> (i32, i32) {
}

fn H() -> i32 {
return G()[0];
return G().0;
}

// CHECK:STDOUT: --- in_place_tuple_init.carbon
Expand Down Expand Up @@ -128,8 +128,8 @@ fn H() -> i32 {
// CHECK:STDOUT: %G.call: init %.3 = call %G.ref() to %.loc20_11.1
// CHECK:STDOUT: %.loc20_14: i32 = int_literal 0 [template = constants.%.5]
// CHECK:STDOUT: %.loc20_11.2: ref %.3 = temporary %.loc20_11.1, %G.call
// CHECK:STDOUT: %.loc20_15.1: ref i32 = tuple_index %.loc20_11.2, %.loc20_14
// CHECK:STDOUT: %.loc20_15.2: i32 = bind_value %.loc20_15.1
// CHECK:STDOUT: return %.loc20_15.2
// CHECK:STDOUT: %.loc20_13.1: ref i32 = tuple_index %.loc20_11.2, %.loc20_14
// CHECK:STDOUT: %.loc20_13.2: i32 = bind_value %.loc20_13.1
// CHECK:STDOUT: return %.loc20_13.2
// CHECK:STDOUT: }
// CHECK:STDOUT:
10 changes: 5 additions & 5 deletions toolchain/check/testdata/function/call/more_param_ir.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn Foo(a: i32, b: i32) {}
fn Main() {
var x: (i32,) = (1,);
// Generates multiple IR instructions for the first parameter.
Foo(x[0], 6);
Foo(x.0, 6);
}

// CHECK:STDOUT: --- more_param_ir.carbon
Expand Down Expand Up @@ -94,10 +94,10 @@ fn Main() {
// CHECK:STDOUT: %Foo.ref: %Foo.type = name_ref Foo, file.%Foo.decl [template = constants.%Foo]
// CHECK:STDOUT: %x.ref: ref %.3 = name_ref x, %x
// CHECK:STDOUT: %.loc16_9: i32 = int_literal 0 [template = constants.%.5]
// CHECK:STDOUT: %.loc16_10.1: ref i32 = tuple_index %x.ref, %.loc16_9
// CHECK:STDOUT: %.loc16_13: i32 = int_literal 6 [template = constants.%.6]
// CHECK:STDOUT: %.loc16_10.2: i32 = bind_value %.loc16_10.1
// CHECK:STDOUT: %Foo.call: init %.1 = call %Foo.ref(%.loc16_10.2, %.loc16_13)
// CHECK:STDOUT: %.loc16_8.1: ref i32 = tuple_index %x.ref, %.loc16_9
// CHECK:STDOUT: %.loc16_12: i32 = int_literal 6 [template = constants.%.6]
// CHECK:STDOUT: %.loc16_8.2: i32 = bind_value %.loc16_8.1
// CHECK:STDOUT: %Foo.call: init %.1 = call %Foo.ref(%.loc16_8.2, %.loc16_12)
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
12 changes: 6 additions & 6 deletions toolchain/check/testdata/function/definition/import.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ library "fns";

fn A() {}
fn B(b: i32) -> i32 { return b; }
fn C(c: (i32,)) -> {.c: i32} { return {.c = c[0]}; }
fn C(c: (i32,)) -> {.c: i32} { return {.c = c.0}; }
fn D();

// --- extern.carbon
Expand Down Expand Up @@ -183,11 +183,11 @@ fn D() {}
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %c.ref: %.3 = name_ref c, %c
// CHECK:STDOUT: %.loc6_47: i32 = int_literal 0 [template = constants.%.5]
// CHECK:STDOUT: %.loc6_48: i32 = tuple_index %c.ref, %.loc6_47
// CHECK:STDOUT: %.loc6_49: %.4 = struct_literal (%.loc6_48)
// CHECK:STDOUT: %struct: %.4 = struct_value (%.loc6_48)
// CHECK:STDOUT: %.loc6_50: %.4 = converted %.loc6_49, %struct
// CHECK:STDOUT: return %.loc6_50
// CHECK:STDOUT: %.loc6_46: i32 = tuple_index %c.ref, %.loc6_47
// CHECK:STDOUT: %.loc6_48: %.4 = struct_literal (%.loc6_46)
// CHECK:STDOUT: %struct: %.4 = struct_value (%.loc6_46)
// CHECK:STDOUT: %.loc6_49: %.4 = converted %.loc6_48, %struct
// CHECK:STDOUT: return %.loc6_49
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @D();
Expand Down
8 changes: 4 additions & 4 deletions toolchain/check/testdata/index/fail_empty_tuple_access.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ fn F() {}

fn Run() {
// CHECK:STDERR: fail_empty_tuple_access.carbon:[[@LINE+3]]:3: ERROR: Tuple element index `0` is past the end of type `()`.
// CHECK:STDERR: F()[0];
// CHECK:STDERR: ^~~~~~
F()[0];
// CHECK:STDERR: F().0;
// CHECK:STDERR: ^~~~~
F().0;
}

// CHECK:STDOUT: --- fail_empty_tuple_access.carbon
Expand Down Expand Up @@ -63,7 +63,7 @@ fn Run() {
// CHECK:STDOUT: %.loc17_7: i32 = int_literal 0 [template = constants.%.2]
// CHECK:STDOUT: %.loc17_4.1: ref %.1 = temporary_storage
// CHECK:STDOUT: %.loc17_4.2: ref %.1 = temporary %.loc17_4.1, %F.call
// CHECK:STDOUT: %.loc17_8: ref <error> = tuple_index %.loc17_4.2, <error> [template = <error>]
// CHECK:STDOUT: %.loc17_6: ref <error> = tuple_index %.loc17_4.2, <error> [template = <error>]
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
21 changes: 12 additions & 9 deletions toolchain/check/testdata/index/fail_negative_indexing.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/index/fail_negative_indexing.carbon

var a: (i32, i32) = (12, 6);
// CHECK:STDERR: fail_negative_indexing.carbon:[[@LINE+3]]:16: ERROR: Cannot access member of interface Negate in type i32 that does not implement that interface.
// CHECK:STDERR: var b: i32 = a[-10];
// CHECK:STDERR: ^~~
var b: i32 = a[-10];
// CHECK:STDERR: fail_negative_indexing.carbon:[[@LINE+7]]:14: ERROR: Member name of type `<error>` in compound member access is not an instance member or an interface member.
// CHECK:STDERR: var b: i32 = a.(-10);
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
// CHECK:STDERR: fail_negative_indexing.carbon:[[@LINE+3]]:17: ERROR: Cannot access member of interface Negate in type i32 that does not implement that interface.
// CHECK:STDERR: var b: i32 = a.(-10);
// CHECK:STDERR: ^~~
var b: i32 = a.(-10);

// CHECK:STDOUT: --- fail_negative_indexing.carbon
// CHECK:STDOUT:
Expand Down Expand Up @@ -72,9 +76,9 @@ var b: i32 = a[-10];
// CHECK:STDOUT: %.loc11_17.6: type = converted %.loc11_17.1, constants.%.3 [template = constants.%.3]
// CHECK:STDOUT: %a.var: ref %.3 = var a
// CHECK:STDOUT: %a: ref %.3 = bind_name a, %a.var
// CHECK:STDOUT: %int.make_type_32.loc15: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc15_8.1: type = value_of_initializer %int.make_type_32.loc15 [template = i32]
// CHECK:STDOUT: %.loc15_8.2: type = converted %int.make_type_32.loc15, %.loc15_8.1 [template = i32]
// CHECK:STDOUT: %int.make_type_32.loc19: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc19_8.1: type = value_of_initializer %int.make_type_32.loc19 [template = i32]
// CHECK:STDOUT: %.loc19_8.2: type = converted %int.make_type_32.loc19, %.loc19_8.1 [template = i32]
// CHECK:STDOUT: %b.var: ref i32 = var b
// CHECK:STDOUT: %b: ref i32 = bind_name b, %b.var
// CHECK:STDOUT: }
Expand Down Expand Up @@ -107,8 +111,7 @@ var b: i32 = a[-10];
// CHECK:STDOUT: %.loc11_28: init %.3 = converted %.loc11_27.1, %.loc11_27.6 [template = constants.%tuple]
// CHECK:STDOUT: assign file.%a.var, %.loc11_28
// CHECK:STDOUT: %a.ref: ref %.3 = name_ref a, file.%a
// CHECK:STDOUT: %.loc15_17: i32 = int_literal 10 [template = constants.%.7]
// CHECK:STDOUT: %.loc15_19: ref <error> = tuple_index %a.ref, <error> [template = <error>]
// CHECK:STDOUT: %.loc19: i32 = int_literal 10 [template = constants.%.7]
// CHECK:STDOUT: assign file.%b.var, <error>
// CHECK:STDOUT: return
// CHECK:STDOUT: }
Expand Down
Loading

0 comments on commit 8ec0632

Please sign in to comment.