Преглед изворни кода

Stop special-casing empty structs. (#2849)

This removes special-casing of empty structs, handling them as just a regular value instead of a builtin. Note the `{} as Type` is still special-cased.

In lowering, removes the test of calling a function using `{}` because it's missing the proper load/store. This setup notices that error whereas the prior worked due to said special-casing. Fixing this will need to be done as part of generally adding loads for variable uses.
Jon Ross-Perkins пре 2 година
родитељ
комит
76151d1fff

+ 0 - 2
toolchain/driver/testdata/semantics_builtin_nodes.carbon

@@ -21,8 +21,6 @@
 // CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeIntegerType, type: nodeTypeType},
 // CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeFloatingPointType, type: nodeTypeType},
 // CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeStringType, type: nodeTypeType},
-// CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeEmptyStructType, type: nodeTypeType},
-// CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeEmptyStruct, type: nodeEmptyStructType},
 // CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeEmptyTupleType, type: nodeTypeType},
 // CHECK:STDOUT:   {kind: CrossReference, arg0: ir0, arg1: nodeEmptyTuple, type: nodeEmptyTupleType},
 // CHECK:STDOUT: ]

+ 1 - 8
toolchain/lowering/lowering_context.cpp

@@ -57,7 +57,6 @@ auto LoweringContext::LowerBlock(SemanticsNodeBlockId block_id) -> void {
 auto LoweringContext::BuildLoweredNodeAsType(SemanticsNodeId node_id)
     -> llvm::Type* {
   switch (node_id.index) {
-    case SemanticsBuiltinKind::EmptyStructType.AsInt():
     case SemanticsBuiltinKind::EmptyTupleType.AsInt():
       // Represent empty types as empty structs.
       // TODO: Investigate special-casing handling of these so that they can be
@@ -121,13 +120,7 @@ auto LoweringContext::GetLoweredNodeAsValue(SemanticsNodeId node_id)
   // TODO: It might be better to built them at initialization, putting them in
   // every IR even if not used. This is probably a performance decision since it
   // would simplify this function.
-  if (node_id == SemanticsNodeId::BuiltinEmptyStruct) {
-    auto* type = GetLoweredNodeAsType(SemanticsNodeId::BuiltinEmptyStructType);
-    auto* value = llvm::ConstantStruct::get(llvm::cast<llvm::StructType>(type),
-                                            llvm::ArrayRef<llvm::Constant*>());
-    node = value;
-    return value;
-  } else if (node_id == SemanticsNodeId::BuiltinEmptyTuple) {
+  if (node_id == SemanticsNodeId::BuiltinEmptyTuple) {
     auto* type = GetLoweredNodeAsType(SemanticsNodeId::BuiltinEmptyTupleType);
     auto* value = llvm::ConstantStruct::get(llvm::cast<llvm::StructType>(type),
                                             llvm::ArrayRef<llvm::Constant*>());

+ 0 - 6
toolchain/lowering/lowering_handle.cpp

@@ -21,12 +21,6 @@ auto LoweringHandleCrossReference(LoweringContext& /*context*/,
 auto LoweringHandleAssign(LoweringContext& context, SemanticsNodeId /*node_id*/,
                           SemanticsNode node) -> void {
   auto [storage_id, value_id] = node.GetAsAssign();
-  if (value_id == SemanticsNodeId::BuiltinEmptyStruct ||
-      value_id == SemanticsNodeId::BuiltinEmptyTuple) {
-    // Elide the 0-length store; these have no value assigned and it should have
-    // no effect.
-    return;
-  }
   context.builder().CreateStore(context.GetLoweredNodeAsValue(value_id),
                                 context.GetLoweredNodeAsValue(storage_id));
 }

+ 2 - 11
toolchain/lowering/testdata/function/call/empty_struct.carbon → toolchain/lowering/testdata/function/definition/empty_struct.carbon

@@ -7,20 +7,11 @@
 // CHECK:STDOUT: source_filename = "empty_struct.carbon"
 // CHECK:STDOUT:
 // CHECK:STDOUT: %EmptyTupleType = type {}
-// CHECK:STDOUT: %EmptyStructType = type {}
+// CHECK:STDOUT: %StructLiteralType = type {}
 // CHECK:STDOUT:
-// CHECK:STDOUT: define %EmptyTupleType @Echo(%EmptyStructType %a) {
+// CHECK:STDOUT: define %EmptyTupleType @Echo(%StructLiteralType %a) {
 // CHECK:STDOUT: entry:
 // CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: define %EmptyTupleType @Main() {
-// CHECK:STDOUT: entry:
-// CHECK:STDOUT:   %Echo = call %EmptyTupleType @Echo(%EmptyStructType zeroinitializer)
-// CHECK:STDOUT: }
 
 fn Echo(a: {}) {
 }
-
-fn Main() {
-  Echo({});
-}

+ 10 - 4
toolchain/lowering/testdata/struct/empty.carbon

@@ -6,13 +6,19 @@
 // CHECK:STDOUT: ; ModuleID = 'empty.carbon'
 // CHECK:STDOUT: source_filename = "empty.carbon"
 // CHECK:STDOUT:
-// CHECK:STDOUT: %EmptyStructType = type {}
+// CHECK:STDOUT: %StructLiteralType = type {}
+// CHECK:STDOUT: %StructLiteralType.0 = type {}
+// CHECK:STDOUT: %StructLiteralType.1 = type {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: define i32 @Run() {
 // CHECK:STDOUT: entry:
-// CHECK:STDOUT:   %var = alloca %EmptyStructType, align 8
-// CHECK:STDOUT:   %var1 = alloca %EmptyStructType, align 8
-// CHECK:STDOUT:   store ptr %var, ptr %var1, align 8
+// CHECK:STDOUT:   %StructLiteralValue = alloca %StructLiteralType, align 8
+// CHECK:STDOUT:   %var = alloca %StructLiteralType, align 8
+// CHECK:STDOUT:   %StructLiteralValue1 = alloca %StructLiteralType.0, align 8
+// CHECK:STDOUT:   store ptr %StructLiteralValue1, ptr %var, align 8
+// CHECK:STDOUT:   %StructLiteralValue2 = alloca %StructLiteralType.1, align 8
+// CHECK:STDOUT:   %var3 = alloca %StructLiteralType.1, align 8
+// CHECK:STDOUT:   store ptr %var, ptr %var3, align 8
 // CHECK:STDOUT:   ret i32 0
 // CHECK:STDOUT: }
 

+ 0 - 6
toolchain/semantics/semantics_builtin_kind.def

@@ -60,12 +60,6 @@ CARBON_SEMANTICS_BUILTIN_KIND(FloatingPointType, TypeType, "f64")
 // The type of string values and String literals.
 CARBON_SEMANTICS_BUILTIN_KIND(StringType, TypeType, "String")
 
-// The canonical empty struct type.
-CARBON_SEMANTICS_BUILTIN_KIND(EmptyStructType, TypeType, "{} as Type")
-
-// The canonical empty struct.
-CARBON_SEMANTICS_BUILTIN_KIND(EmptyStruct, EmptyStructType, "{}")
-
 // The canonical empty tuple type.
 CARBON_SEMANTICS_BUILTIN_KIND(EmptyTupleType, TypeType, "() as Type")
 

+ 10 - 5
toolchain/semantics/semantics_context.cpp

@@ -251,10 +251,12 @@ auto SemanticsContext::ImplicitAsImpl(SemanticsNodeId value_id,
     // If the value is invalid, we can't do much, but do "succeed".
     return ImplicitAsKind::Identical;
   }
-  auto value_type_id = semantics_->GetNode(value_id).type_id();
+  auto value = semantics_->GetNode(value_id);
+  auto value_type_id = value.type_id();
   if (value_type_id == SemanticsNodeId::BuiltinInvalidType) {
     return ImplicitAsKind::Identical;
   }
+
   if (as_type_id == SemanticsNodeId::BuiltinInvalidType) {
     // Although the target type is invalid, this still changes the value.
     if (output_value_id != nullptr) {
@@ -268,18 +270,21 @@ auto SemanticsContext::ImplicitAsImpl(SemanticsNodeId value_id,
     return ImplicitAsKind::Identical;
   }
 
-  // When converting to a Type, there are some automatic conversions that can be
-  // done.
   if (as_type_id == SemanticsNodeId::BuiltinTypeType) {
+    // When converting `()` to a type, the result is `() as Type`.
+    // TODO: This might switch to be closer to the struct conversion below.
     if (value_id == SemanticsNodeId::BuiltinEmptyTuple) {
       if (output_value_id != nullptr) {
         *output_value_id = SemanticsNodeId::BuiltinEmptyTupleType;
       }
       return ImplicitAsKind::Compatible;
     }
-    if (value_id == SemanticsNodeId::BuiltinEmptyStruct) {
+
+    // When converting `{}` to a type, the result is `{} as Type`.
+    if (value.kind() == SemanticsNodeKind::StructValue &&
+        value.GetAsStructValue() == SemanticsNodeBlockId::Empty) {
       if (output_value_id != nullptr) {
-        *output_value_id = SemanticsNodeId::BuiltinEmptyStructType;
+        *output_value_id = value_type_id;
       }
       return ImplicitAsKind::Compatible;
     }

+ 0 - 6
toolchain/semantics/semantics_handle_struct.cpp

@@ -74,12 +74,6 @@ auto SemanticsHandleStructLiteral(SemanticsContext& context,
       ParseNodeKind::StructLiteralOrStructTypeLiteralStart);
   auto type_block_id = context.args_type_info_stack().Pop();
 
-  // Special-case `{}`.
-  if (refs_id == SemanticsNodeBlockId::Empty) {
-    context.node_stack().Push(parse_node, SemanticsNodeId::BuiltinEmptyStruct);
-    return true;
-  }
-
   // Construct a type for the literal.
   // TODO: This should try to canonicalize the struct form before adding the
   // node.

+ 4 - 1
toolchain/semantics/semantics_ir.cpp

@@ -166,7 +166,10 @@ auto SemanticsIR::StringifyNode(SemanticsNodeId node_id) -> std::string {
     switch (node.kind()) {
       case SemanticsNodeKind::StructType: {
         auto refs = GetNodeBlock(node.GetAsStructType());
-        if (step.index == 0) {
+        if (refs.empty()) {
+          out << "{} as Type";
+          break;
+        } else if (step.index == 0) {
           out << "{";
         } else if (step.index < static_cast<int>(refs.size())) {
           out << ", ";

+ 29 - 15
toolchain/semantics/testdata/function/call/empty_struct.carbon

@@ -5,7 +5,7 @@
 // AUTOUPDATE
 // CHECK:STDOUT: cross_reference_irs_size: 1
 // CHECK:STDOUT: callables: [
-// CHECK:STDOUT:   {param_refs: block2, return_type: nodeEmptyStructType},
+// CHECK:STDOUT:   {param_refs: block2, return_type: node+4},
 // CHECK:STDOUT:   {param_refs: block0},
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: integer_literals: [
@@ -18,18 +18,26 @@
 // CHECK:STDOUT:   Main,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: types: [
-// CHECK:STDOUT:   nodeEmptyStructType,
+// CHECK:STDOUT:   node+0,
+// CHECK:STDOUT:   node+4,
+// CHECK:STDOUT:   node+10,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: nodes: [
-// CHECK:STDOUT:   {kind: VarStorage, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: BindName, arg0: str0, arg1: node+0, type: nodeEmptyStructType},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+0},
+// CHECK:STDOUT:   {kind: VarStorage, type: node+0},
+// CHECK:STDOUT:   {kind: BindName, arg0: str0, arg1: node+2, type: node+0},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+4},
 // CHECK:STDOUT:   {kind: FunctionDeclaration, arg0: str1, arg1: callable0},
-// CHECK:STDOUT:   {kind: ReturnExpression, arg0: node+0, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: FunctionDefinition, arg0: node+2, arg1: block4},
+// CHECK:STDOUT:   {kind: ReturnExpression, arg0: node+2, type: node+0},
+// CHECK:STDOUT:   {kind: FunctionDefinition, arg0: node+6, arg1: block4},
 // CHECK:STDOUT:   {kind: FunctionDeclaration, arg0: str2, arg1: callable1},
-// CHECK:STDOUT:   {kind: StubReference, arg0: nodeEmptyStruct, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: Call, arg0: block6, arg1: callable0, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: FunctionDefinition, arg0: node+5, arg1: block5},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+10},
+// CHECK:STDOUT:   {kind: StubReference, arg0: node+11, type: node+10},
+// CHECK:STDOUT:   {kind: Call, arg0: block6, arg1: callable0, type: node+4},
+// CHECK:STDOUT:   {kind: FunctionDefinition, arg0: node+9, arg1: block5},
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: node_blocks: [
 // CHECK:STDOUT:   [
@@ -37,25 +45,31 @@
 // CHECK:STDOUT:   [
 // CHECK:STDOUT:     node+0,
 // CHECK:STDOUT:     node+1,
+// CHECK:STDOUT:     node+2,
+// CHECK:STDOUT:     node+3,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+1,
+// CHECK:STDOUT:     node+3,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+2,
 // CHECK:STDOUT:     node+4,
 // CHECK:STDOUT:     node+5,
+// CHECK:STDOUT:     node+6,
 // CHECK:STDOUT:     node+8,
+// CHECK:STDOUT:     node+9,
+// CHECK:STDOUT:     node+14,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+3,
+// CHECK:STDOUT:     node+7,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+6,
-// CHECK:STDOUT:     node+7,
+// CHECK:STDOUT:     node+10,
+// CHECK:STDOUT:     node+11,
+// CHECK:STDOUT:     node+12,
+// CHECK:STDOUT:     node+13,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+6,
+// CHECK:STDOUT:     node+12,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT: ]
 

+ 21 - 7
toolchain/semantics/testdata/struct/empty.carbon

@@ -15,15 +15,23 @@
 // CHECK:STDOUT:   y,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: types: [
-// CHECK:STDOUT:   nodeEmptyStructType,
+// CHECK:STDOUT:   node+0,
+// CHECK:STDOUT:   node+4,
+// CHECK:STDOUT:   node+7,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: nodes: [
-// CHECK:STDOUT:   {kind: VarStorage, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: BindName, arg0: str0, arg1: node+0, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: Assign, arg0: node+0, arg1: nodeEmptyStruct, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: VarStorage, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: BindName, arg0: str1, arg1: node+3, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: Assign, arg0: node+3, arg1: node+0, type: nodeEmptyStructType},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+0},
+// CHECK:STDOUT:   {kind: VarStorage, type: node+0},
+// CHECK:STDOUT:   {kind: BindName, arg0: str0, arg1: node+2, type: node+0},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+4},
+// CHECK:STDOUT:   {kind: Assign, arg0: node+2, arg1: node+5, type: node+4},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+7},
+// CHECK:STDOUT:   {kind: VarStorage, type: node+7},
+// CHECK:STDOUT:   {kind: BindName, arg0: str1, arg1: node+9, type: node+7},
+// CHECK:STDOUT:   {kind: Assign, arg0: node+9, arg1: node+2, type: node+0},
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: node_blocks: [
 // CHECK:STDOUT:   [
@@ -35,6 +43,12 @@
 // CHECK:STDOUT:     node+3,
 // CHECK:STDOUT:     node+4,
 // CHECK:STDOUT:     node+5,
+// CHECK:STDOUT:     node+6,
+// CHECK:STDOUT:     node+7,
+// CHECK:STDOUT:     node+8,
+// CHECK:STDOUT:     node+9,
+// CHECK:STDOUT:     node+10,
+// CHECK:STDOUT:     node+11,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT: ]
 

+ 5 - 0
toolchain/semantics/testdata/struct/fail_assign_empty.carbon

@@ -17,12 +17,15 @@
 // CHECK:STDOUT: types: [
 // CHECK:STDOUT:   nodeIntegerType,
 // CHECK:STDOUT:   node+1,
+// CHECK:STDOUT:   node+4,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: nodes: [
 // CHECK:STDOUT:   {kind: StructTypeField, arg0: str0, type: nodeIntegerType},
 // CHECK:STDOUT:   {kind: StructType, arg0: block2, type: nodeTypeType},
 // CHECK:STDOUT:   {kind: VarStorage, type: node+1},
 // CHECK:STDOUT:   {kind: BindName, arg0: str1, arg1: node+2, type: node+1},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+4},
 // CHECK:STDOUT:   {kind: Assign, arg0: node+2, arg1: nodeInvalidType, type: nodeInvalidType},
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: node_blocks: [
@@ -34,6 +37,8 @@
 // CHECK:STDOUT:     node+2,
 // CHECK:STDOUT:     node+3,
 // CHECK:STDOUT:     node+4,
+// CHECK:STDOUT:     node+5,
+// CHECK:STDOUT:     node+6,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
 // CHECK:STDOUT:     node+0,

+ 14 - 10
toolchain/semantics/testdata/struct/fail_assign_to_empty.carbon

@@ -16,19 +16,21 @@
 // CHECK:STDOUT:   a,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: types: [
-// CHECK:STDOUT:   nodeEmptyStructType,
+// CHECK:STDOUT:   node+0,
 // CHECK:STDOUT:   nodeIntegerType,
-// CHECK:STDOUT:   node+5,
+// CHECK:STDOUT:   node+7,
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: nodes: [
-// CHECK:STDOUT:   {kind: VarStorage, type: nodeEmptyStructType},
-// CHECK:STDOUT:   {kind: BindName, arg0: str0, arg1: node+0, type: nodeEmptyStructType},
+// CHECK:STDOUT:   {kind: StructType, arg0: block0, type: nodeTypeType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block0, type: node+0},
+// CHECK:STDOUT:   {kind: VarStorage, type: node+0},
+// CHECK:STDOUT:   {kind: BindName, arg0: str0, arg1: node+2, type: node+0},
 // CHECK:STDOUT:   {kind: IntegerLiteral, arg0: int0, type: nodeIntegerType},
 // CHECK:STDOUT:   {kind: StructTypeField, arg0: str1, type: nodeIntegerType},
-// CHECK:STDOUT:   {kind: StubReference, arg0: node+2, type: nodeIntegerType},
+// CHECK:STDOUT:   {kind: StubReference, arg0: node+4, type: nodeIntegerType},
 // CHECK:STDOUT:   {kind: StructType, arg0: block2, type: nodeTypeType},
-// CHECK:STDOUT:   {kind: StructValue, arg0: block3, type: node+5},
-// CHECK:STDOUT:   {kind: Assign, arg0: node+0, arg1: nodeInvalidType, type: nodeInvalidType},
+// CHECK:STDOUT:   {kind: StructValue, arg0: block3, type: node+7},
+// CHECK:STDOUT:   {kind: Assign, arg0: node+2, arg1: nodeInvalidType, type: nodeInvalidType},
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: node_blocks: [
 // CHECK:STDOUT:   [
@@ -37,16 +39,18 @@
 // CHECK:STDOUT:     node+0,
 // CHECK:STDOUT:     node+1,
 // CHECK:STDOUT:     node+2,
+// CHECK:STDOUT:     node+3,
 // CHECK:STDOUT:     node+4,
-// CHECK:STDOUT:     node+5,
 // CHECK:STDOUT:     node+6,
 // CHECK:STDOUT:     node+7,
+// CHECK:STDOUT:     node+8,
+// CHECK:STDOUT:     node+9,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+3,
+// CHECK:STDOUT:     node+5,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT:   [
-// CHECK:STDOUT:     node+4,
+// CHECK:STDOUT:     node+6,
 // CHECK:STDOUT:   ],
 // CHECK:STDOUT: ]