Refactor index logic to share more code. (#3152)

Trying to improve diagnostic names and code flow. I was thinking
"operand" might be a better name than "name" in the context of what `a`
is in `a[b]`, where we already call `b` the `index`.

Flow-wise, note this replaces pushing BuiltinError with instead creating
Index nodes that may contain an error. This is intended to aim towards
producing a more consistent SemIR in the face of errors. There are pros
and cons of both approaches, but I'm aiming for simpler control flow.
This commit is contained in:
Jon Ross-Perkins
2023-08-25 22:31:26 +00:00
committed by GitHub
parent 3eb2e6658f
commit 2d586de63c
11 changed files with 126 additions and 79 deletions
+3 -3
View File
@@ -112,9 +112,9 @@ CARBON_DIAGNOSTIC_KIND(CallArgTypeMismatch)
CARBON_DIAGNOSTIC_KIND(MissingReturnStatement)
CARBON_DIAGNOSTIC_KIND(RepeatedConst)
CARBON_DIAGNOSTIC_KIND(InvalidArrayExpression)
CARBON_DIAGNOSTIC_KIND(InvalidIndexExpression)
CARBON_DIAGNOSTIC_KIND(NondeterministicType)
CARBON_DIAGNOSTIC_KIND(OutOfBoundsAccess)
CARBON_DIAGNOSTIC_KIND(TypeNotIndexable)
CARBON_DIAGNOSTIC_KIND(IndexOutOfBounds)
CARBON_DIAGNOSTIC_KIND(TupleIndexIntegerLiteral)
CARBON_DIAGNOSTIC_KIND(ReturnStatementDisallowExpression)
CARBON_DIAGNOSTIC_KIND(ReturnStatementImplicitNote)
CARBON_DIAGNOSTIC_KIND(ReturnStatementMissingExpression)
+75 -61
View File
@@ -15,79 +15,93 @@ auto HandleIndexExpressionStart(Context& /*context*/,
return true;
}
// Validates that the index (required to be an IntegerLiteral) is valid within
// the array or tuple size. Returns the index on success, or nullptr on failure.
static auto ValidateIntegerLiteralBound(Context& context,
ParseTree::Node parse_node,
SemIR::Node operand_node,
SemIR::Node index_node, int size)
-> const llvm::APInt* {
const auto& index_val = context.semantics_ir().GetIntegerLiteral(
index_node.GetAsIntegerLiteral());
if (index_val.uge(size)) {
CARBON_DIAGNOSTIC(IndexOutOfBounds, Error,
"Index `{0}` is past the end of `{1}`.", llvm::APSInt,
std::string);
context.emitter().Emit(
parse_node, IndexOutOfBounds,
llvm::APSInt(index_val, /*isUnsigned=*/true),
context.semantics_ir().StringifyType(operand_node.type_id()));
return nullptr;
}
return &index_val;
}
auto HandleIndexExpression(Context& context, ParseTree::Node parse_node)
-> bool {
CARBON_DIAGNOSTIC(OutOfBoundsAccess, Error,
"Index `{0}` is past the end of `{1}`.", llvm::APSInt,
std::string);
auto index_node_id = context.node_stack().PopExpression();
auto index_node = context.semantics_ir().GetNode(index_node_id);
auto name_node_id = context.node_stack().PopExpression();
name_node_id = context.MaterializeIfInitializing(name_node_id);
auto name_node = context.semantics_ir().GetNode(name_node_id);
auto name_type_id =
context.semantics_ir().GetTypeAllowBuiltinTypes(name_node.type_id());
auto name_type_node = context.semantics_ir().GetNode(name_type_id);
auto operand_node_id = context.node_stack().PopExpression();
operand_node_id = context.MaterializeIfInitializing(operand_node_id);
auto operand_node = context.semantics_ir().GetNode(operand_node_id);
auto operand_type_id = operand_node.type_id();
auto operand_type_node = context.semantics_ir().GetNode(
context.semantics_ir().GetTypeAllowBuiltinTypes(operand_type_id));
if (name_type_node.kind() == SemIR::NodeKind::ArrayType) {
auto [bound_id, type_id] = name_type_node.GetAsArrayType();
if (index_node.kind() == SemIR::NodeKind::IntegerLiteral) {
const auto& index_val = context.semantics_ir().GetIntegerLiteral(
index_node.GetAsIntegerLiteral());
if (index_val.uge(context.semantics_ir().GetArrayBoundValue(bound_id))) {
context.emitter().Emit(
parse_node, OutOfBoundsAccess,
llvm::APSInt(index_val, /*isUnsigned=*/true),
context.semantics_ir().StringifyType(name_node.type_id()));
} else {
context.AddNodeAndPush(
parse_node, SemIR::Node::ArrayIndex::Make(
parse_node, type_id, name_node_id, index_node_id));
return true;
switch (operand_type_node.kind()) {
case SemIR::NodeKind::ArrayType: {
auto [bound_id, element_type_id] = operand_type_node.GetAsArrayType();
// We can check whether integers are in-bounds, although it doesn't affect
// the IR for an array.
if (index_node.kind() == SemIR::NodeKind::IntegerLiteral &&
!ValidateIntegerLiteralBound(
context, parse_node, operand_node, index_node,
context.semantics_ir().GetArrayBoundValue(bound_id))) {
index_node_id = SemIR::NodeId::BuiltinError;
}
} else if (context.ImplicitAsRequired(
index_node.parse_node(), index_node_id,
context.CanonicalizeType(
SemIR::NodeId::BuiltinIntegerType)) !=
SemIR::NodeId::BuiltinError) {
context.AddNodeAndPush(
parse_node, SemIR::Node::ArrayIndex::Make(
parse_node, type_id, name_node_id, index_node_id));
auto cast_index_id = context.ImplicitAsRequired(
index_node.parse_node(), index_node_id,
context.CanonicalizeType(SemIR::NodeId::BuiltinIntegerType));
context.AddNodeAndPush(parse_node, SemIR::Node::ArrayIndex::Make(
parse_node, element_type_id,
operand_node_id, cast_index_id));
return true;
}
} else if (name_type_node.kind() == SemIR::NodeKind::TupleType) {
if (index_node.kind() == SemIR::NodeKind::IntegerLiteral) {
const auto& index_val = context.semantics_ir().GetIntegerLiteral(
index_node.GetAsIntegerLiteral());
auto type_block =
context.semantics_ir().GetTypeBlock(name_type_node.GetAsTupleType());
if (index_val.uge(static_cast<uint64_t>(type_block.size()))) {
context.emitter().Emit(
parse_node, OutOfBoundsAccess,
llvm::APSInt(index_val, /*isUnsigned=*/true),
context.semantics_ir().StringifyType(name_node.type_id()));
case SemIR::NodeKind::TupleType: {
SemIR::TypeId element_type_id = SemIR::TypeId::Error;
if (index_node.kind() == SemIR::NodeKind::IntegerLiteral) {
auto type_block = context.semantics_ir().GetTypeBlock(
operand_type_node.GetAsTupleType());
if (const auto* index_val =
ValidateIntegerLiteralBound(context, parse_node, operand_node,
index_node, type_block.size())) {
element_type_id = type_block[index_val->getZExtValue()];
} else {
index_node_id = SemIR::NodeId::BuiltinError;
}
} else {
context.AddNodeAndPush(
parse_node, SemIR::Node::TupleIndex::Make(
parse_node, type_block[index_val.getZExtValue()],
name_node_id, index_node_id));
return true;
CARBON_DIAGNOSTIC(TupleIndexIntegerLiteral, Error,
"Tuples indices must be integer literals.");
context.emitter().Emit(parse_node, TupleIndexIntegerLiteral);
index_node_id = SemIR::NodeId::BuiltinError;
}
} else {
CARBON_DIAGNOSTIC(NondeterministicType, Error,
"Type cannot be determined at compile time.");
context.emitter().Emit(parse_node, NondeterministicType);
context.AddNodeAndPush(parse_node, SemIR::Node::TupleIndex::Make(
parse_node, element_type_id,
operand_node_id, index_node_id));
return true;
}
default: {
if (operand_type_id != SemIR::TypeId::Error) {
CARBON_DIAGNOSTIC(TypeNotIndexable, Error,
"`{0}` does not support indexing.", std::string);
context.emitter().Emit(
parse_node, TypeNotIndexable,
context.semantics_ir().StringifyType(operand_type_id));
}
context.node_stack().Push(parse_node, SemIR::NodeId::BuiltinError);
return true;
}
} else if (name_type_id != SemIR::NodeId::BuiltinError) {
CARBON_DIAGNOSTIC(InvalidIndexExpression, Error,
"Invalid index expression.");
context.emitter().Emit(parse_node, InvalidIndexExpression);
}
context.node_stack().Push(parse_node, SemIR::NodeId::BuiltinError);
return true;
}
} // namespace Carbon::Check
@@ -46,7 +46,9 @@ var b: i32 = a[0xFFFFFFFFFFFFFFFFF];
// CHECK:STDOUT: {kind: Assign, arg0: node+2, arg1: node+7},
// CHECK:STDOUT: {kind: VarStorage, arg0: str1, type: type0},
// CHECK:STDOUT: {kind: IntegerLiteral, arg0: int2, type: type0},
// CHECK:STDOUT: {kind: Assign, arg0: node+9, arg1: nodeError},
// CHECK:STDOUT: {kind: ArrayIndex, arg0: node+2, arg1: nodeError, type: type0},
// CHECK:STDOUT: {kind: BindValue, arg0: node+11, type: type0},
// CHECK:STDOUT: {kind: Assign, arg0: node+9, arg1: node+12},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
// CHECK:STDOUT: [
@@ -64,6 +66,8 @@ var b: i32 = a[0xFFFFFFFFFFFFFFFFF];
// CHECK:STDOUT: node+9,
// CHECK:STDOUT: node+10,
// CHECK:STDOUT: node+11,
// CHECK:STDOUT: node+12,
// CHECK:STDOUT: node+13,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+4,
@@ -81,6 +85,8 @@ var b: i32 = a[0xFFFFFFFFFFFFFFFFF];
// CHECK:STDOUT: %.loc7_23.3: [i32; 1] = array_value %.loc7_23.2
// CHECK:STDOUT: assign %a, %.loc7_23.3
// CHECK:STDOUT: %b: i32 = var "b"
// CHECK:STDOUT: %.loc11: i32 = int_literal -1
// CHECK:STDOUT: assign %b, <error>
// CHECK:STDOUT: %.loc11_16: i32 = int_literal -1
// CHECK:STDOUT: %.loc11_35.1: i32 = array_index %a, <error>
// CHECK:STDOUT: %.loc11_35.2: i32 = bind_value %.loc11_35.1
// CHECK:STDOUT: assign %b, %.loc11_35.2
// CHECK:STDOUT: }
@@ -47,7 +47,9 @@ var b: i32 = a[2.6];
// CHECK:STDOUT: {kind: Assign, arg0: node+2, arg1: node+7},
// CHECK:STDOUT: {kind: VarStorage, arg0: str1, type: type0},
// CHECK:STDOUT: {kind: RealLiteral, arg0: real0, type: type3},
// CHECK:STDOUT: {kind: Assign, arg0: node+9, arg1: nodeError},
// CHECK:STDOUT: {kind: ArrayIndex, arg0: node+2, arg1: nodeError, type: type0},
// CHECK:STDOUT: {kind: BindValue, arg0: node+11, type: type0},
// CHECK:STDOUT: {kind: Assign, arg0: node+9, arg1: node+12},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
// CHECK:STDOUT: [
@@ -65,6 +67,8 @@ var b: i32 = a[2.6];
// CHECK:STDOUT: node+9,
// CHECK:STDOUT: node+10,
// CHECK:STDOUT: node+11,
// CHECK:STDOUT: node+12,
// CHECK:STDOUT: node+13,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+4,
@@ -82,6 +86,8 @@ var b: i32 = a[2.6];
// CHECK:STDOUT: %.loc7_23.3: [i32; 1] = array_value %.loc7_23.2
// CHECK:STDOUT: assign %a, %.loc7_23.3
// CHECK:STDOUT: %b: i32 = var "b"
// CHECK:STDOUT: %.loc11: f64 = real_literal 26e-1
// CHECK:STDOUT: assign %b, <error>
// CHECK:STDOUT: %.loc11_16: f64 = real_literal 26e-1
// CHECK:STDOUT: %.loc11_19.1: i32 = array_index %a, <error>
// CHECK:STDOUT: %.loc11_19.2: i32 = bind_value %.loc11_19.1
// CHECK:STDOUT: assign %b, %.loc11_19.2
// CHECK:STDOUT: }
@@ -46,7 +46,9 @@ var b: i32 = a[2];
// CHECK:STDOUT: {kind: Assign, arg0: node+2, arg1: node+7},
// CHECK:STDOUT: {kind: VarStorage, arg0: str1, type: type0},
// CHECK:STDOUT: {kind: IntegerLiteral, arg0: int2, type: type0},
// CHECK:STDOUT: {kind: Assign, arg0: node+9, arg1: nodeError},
// CHECK:STDOUT: {kind: ArrayIndex, arg0: node+2, arg1: nodeError, type: type0},
// CHECK:STDOUT: {kind: BindValue, arg0: node+11, type: type0},
// CHECK:STDOUT: {kind: Assign, arg0: node+9, arg1: node+12},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
// CHECK:STDOUT: [
@@ -64,6 +66,8 @@ var b: i32 = a[2];
// CHECK:STDOUT: node+9,
// CHECK:STDOUT: node+10,
// CHECK:STDOUT: node+11,
// CHECK:STDOUT: node+12,
// CHECK:STDOUT: node+13,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+4,
@@ -81,6 +85,8 @@ var b: i32 = a[2];
// CHECK:STDOUT: %.loc7_23.3: [i32; 1] = array_value %.loc7_23.2
// CHECK:STDOUT: assign %a, %.loc7_23.3
// CHECK:STDOUT: %b: i32 = var "b"
// CHECK:STDOUT: %.loc11: i32 = int_literal 2
// CHECK:STDOUT: assign %b, <error>
// CHECK:STDOUT: %.loc11_16: i32 = int_literal 2
// CHECK:STDOUT: %.loc11_17.1: i32 = array_index %a, <error>
// CHECK:STDOUT: %.loc11_17.2: i32 = bind_value %.loc11_17.1
// CHECK:STDOUT: assign %b, %.loc11_17.2
// CHECK:STDOUT: }
@@ -43,6 +43,7 @@ fn Run() {
// CHECK:STDOUT: {kind: Call, arg0: block0, arg1: function0, type: type0},
// CHECK:STDOUT: {kind: IntegerLiteral, arg0: int0, type: type1},
// CHECK:STDOUT: {kind: MaterializeTemporary, type: type0},
// CHECK:STDOUT: {kind: TupleIndex, arg0: node+6, arg1: nodeError, type: typeError},
// CHECK:STDOUT: {kind: Return},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
@@ -61,6 +62,7 @@ fn Run() {
// CHECK:STDOUT: node+5,
// CHECK:STDOUT: node+6,
// CHECK:STDOUT: node+7,
// CHECK:STDOUT: node+8,
// CHECK:STDOUT: ],
// CHECK:STDOUT: ]
// CHECK:STDOUT:
@@ -80,5 +82,6 @@ fn Run() {
// CHECK:STDOUT: %.loc13_4.2: () = call @F()
// CHECK:STDOUT: %.loc13_7: i32 = int_literal 0
// CHECK:STDOUT: %.loc13_4.3: () = materialize_temporary
// CHECK:STDOUT: %.loc13_8: <error> = tuple_index %.loc13_4.3, <error>
// CHECK:STDOUT: return
// CHECK:STDOUT: }
@@ -6,7 +6,7 @@
var a: (i32, i32) = (2, 3);
var b: i32 = 0;
// CHECK:STDERR: fail_non_deterministic_type.carbon:[[@LINE+3]]:17: Type cannot be determined at compile time.
// CHECK:STDERR: fail_non_deterministic_type.carbon:[[@LINE+3]]:17: Tuples indices must be integer literals.
// CHECK:STDERR: var c: i32 = a[b];
// CHECK:STDERR: ^
var c: i32 = a[b];
@@ -58,6 +58,7 @@ var c: i32 = a[b];
// CHECK:STDOUT: {kind: IntegerLiteral, arg0: int2, type: type1},
// CHECK:STDOUT: {kind: Assign, arg0: node+12, arg1: node+13},
// CHECK:STDOUT: {kind: VarStorage, arg0: str2, type: type1},
// CHECK:STDOUT: {kind: TupleIndex, arg0: node+5, arg1: nodeError, type: typeError},
// CHECK:STDOUT: {kind: Assign, arg0: node+15, arg1: nodeError},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
@@ -81,6 +82,7 @@ var c: i32 = a[b];
// CHECK:STDOUT: node+14,
// CHECK:STDOUT: node+15,
// CHECK:STDOUT: node+16,
// CHECK:STDOUT: node+17,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+0,
@@ -109,5 +111,6 @@ var c: i32 = a[b];
// CHECK:STDOUT: %.loc8: i32 = int_literal 0
// CHECK:STDOUT: assign %b, %.loc8
// CHECK:STDOUT: %c: i32 = var "c"
// CHECK:STDOUT: %.loc12: <error> = tuple_index %a, <error>
// CHECK:STDOUT: assign %c, <error>
// CHECK:STDOUT: }
@@ -5,7 +5,7 @@
// AUTOUPDATE
fn Main() {
// CHECK:STDERR: fail_non_tuple_access.carbon:[[@LINE+3]]:6: Invalid index expression.
// CHECK:STDERR: fail_non_tuple_access.carbon:[[@LINE+3]]:6: `i32` does not support indexing.
// CHECK:STDERR: 0[1];
// CHECK:STDERR: ^
0[1];
@@ -55,6 +55,7 @@ var c: i32 = b[0xFFFFFFFFFFFFFFFFF];
// CHECK:STDOUT: {kind: Assign, arg0: node+11, arg1: node+12},
// CHECK:STDOUT: {kind: VarStorage, arg0: str2, type: type1},
// CHECK:STDOUT: {kind: IntegerLiteral, arg0: int1, type: type1},
// CHECK:STDOUT: {kind: TupleIndex, arg0: node+11, arg1: nodeError, type: typeError},
// CHECK:STDOUT: {kind: Assign, arg0: node+14, arg1: nodeError},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
@@ -78,6 +79,7 @@ var c: i32 = b[0xFFFFFFFFFFFFFFFFF];
// CHECK:STDOUT: node+14,
// CHECK:STDOUT: node+15,
// CHECK:STDOUT: node+16,
// CHECK:STDOUT: node+17,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+0,
@@ -106,6 +108,7 @@ var c: i32 = b[0xFFFFFFFFFFFFFFFFF];
// CHECK:STDOUT: %.loc7_5: (i32,) = bind_value %a
// CHECK:STDOUT: assign %b, %.loc7_5
// CHECK:STDOUT: %c: i32 = var "c"
// CHECK:STDOUT: %.loc12: i32 = int_literal -1
// CHECK:STDOUT: %.loc12_16: i32 = int_literal -1
// CHECK:STDOUT: %.loc12_35: <error> = tuple_index %b, <error>
// CHECK:STDOUT: assign %c, <error>
// CHECK:STDOUT: }
@@ -5,7 +5,7 @@
// AUTOUPDATE
var a: (i32, i32) = (12, 6);
// CHECK:STDERR: fail_tuple_non_int_indexing.carbon:[[@LINE+3]]:19: Type cannot be determined at compile time.
// CHECK:STDERR: fail_tuple_non_int_indexing.carbon:[[@LINE+3]]:19: Tuples indices must be integer literals.
// CHECK:STDERR: var b: i32 = a[2.6];
// CHECK:STDERR: ^
var b: i32 = a[2.6];
@@ -55,6 +55,7 @@ var b: i32 = a[2.6];
// CHECK:STDOUT: {kind: Assign, arg0: node+5, arg1: node+10},
// CHECK:STDOUT: {kind: VarStorage, arg0: str1, type: type1},
// CHECK:STDOUT: {kind: RealLiteral, arg0: real0, type: type3},
// CHECK:STDOUT: {kind: TupleIndex, arg0: node+5, arg1: nodeError, type: typeError},
// CHECK:STDOUT: {kind: Assign, arg0: node+12, arg1: nodeError},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
@@ -76,6 +77,7 @@ var b: i32 = a[2.6];
// CHECK:STDOUT: node+12,
// CHECK:STDOUT: node+13,
// CHECK:STDOUT: node+14,
// CHECK:STDOUT: node+15,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+0,
@@ -101,6 +103,7 @@ var b: i32 = a[2.6];
// CHECK:STDOUT: %.loc7_27: (i32, i32) = tuple_value (%.loc7_22.2, %.loc7_26.2)
// CHECK:STDOUT: assign %a, %.loc7_27
// CHECK:STDOUT: %b: i32 = var "b"
// CHECK:STDOUT: %.loc11: f64 = real_literal 26e-1
// CHECK:STDOUT: %.loc11_16: f64 = real_literal 26e-1
// CHECK:STDOUT: %.loc11_19: <error> = tuple_index %a, <error>
// CHECK:STDOUT: assign %b, <error>
// CHECK:STDOUT: }
@@ -54,6 +54,7 @@ var b: i32 = a[2];
// CHECK:STDOUT: {kind: Assign, arg0: node+5, arg1: node+10},
// CHECK:STDOUT: {kind: VarStorage, arg0: str1, type: type1},
// CHECK:STDOUT: {kind: IntegerLiteral, arg0: int2, type: type1},
// CHECK:STDOUT: {kind: TupleIndex, arg0: node+5, arg1: nodeError, type: typeError},
// CHECK:STDOUT: {kind: Assign, arg0: node+12, arg1: nodeError},
// CHECK:STDOUT: ]
// CHECK:STDOUT: node_blocks: [
@@ -75,6 +76,7 @@ var b: i32 = a[2];
// CHECK:STDOUT: node+12,
// CHECK:STDOUT: node+13,
// CHECK:STDOUT: node+14,
// CHECK:STDOUT: node+15,
// CHECK:STDOUT: ],
// CHECK:STDOUT: [
// CHECK:STDOUT: node+0,
@@ -100,6 +102,7 @@ var b: i32 = a[2];
// CHECK:STDOUT: %.loc7_27: (i32, i32) = tuple_value (%.loc7_22.2, %.loc7_26.2)
// CHECK:STDOUT: assign %a, %.loc7_27
// CHECK:STDOUT: %b: i32 = var "b"
// CHECK:STDOUT: %.loc11: i32 = int_literal 2
// CHECK:STDOUT: %.loc11_16: i32 = int_literal 2
// CHECK:STDOUT: %.loc11_17: <error> = tuple_index %a, <error>
// CHECK:STDOUT: assign %b, <error>
// CHECK:STDOUT: }