Просмотр исходного кода

Emit function definitions in check, for all specifics seen. (#5090)

Emitting definitions in check. This resolves the crash in lowering which
necessitated definitions be emitted.
Some of the test changes need further review.
Alina Sbirlea 1 год назад
Родитель
Сommit
077cf56a8a

+ 0 - 5
toolchain/check/call.cpp

@@ -189,8 +189,6 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
                   context, SemIR::SpecificFunctionType::SingletonInstId),
               .callee_id = generic_callee_id,
               .specific_id = *callee_specific_id});
-      // TODO: Add to `definitions_required` when evaluating the
-      // `SpecificImplFunction`.
     } else {
       // This is a regular generic function. The callee is the specific function
       // we deduced.
@@ -201,9 +199,6 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
                   context, SemIR::SpecificFunctionType::SingletonInstId),
               .callee_id = generic_callee_id,
               .specific_id = *callee_specific_id});
-      // TODO: The specific function could be a symbolic constant. Delay doing
-      // this until we form a concrete `SpecificFunction` constant.
-      context.definitions_required().push_back(callee_id);
     }
 
     // Add the `self` argument back if there was one.

+ 27 - 24
toolchain/check/check_unit.cpp

@@ -20,6 +20,8 @@
 #include "toolchain/check/inst.h"
 #include "toolchain/check/node_id_traversal.h"
 #include "toolchain/check/type.h"
+#include "toolchain/sem_ir/function.h"
+#include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/import_ir.h"
 
 namespace Carbon::Check {
@@ -440,9 +442,11 @@ auto CheckUnit::CheckRequiredDeclarations() -> void {
 auto CheckUnit::CheckRequiredDefinitions() -> void {
   CARBON_DIAGNOSTIC(MissingDefinitionInImpl, Error,
                     "no definition found for declaration in impl file");
+
   // Note that more required definitions can be added during this loop.
-  for (size_t i = 0; i != context_.definitions_required().size(); ++i) {
-    SemIR::InstId decl_inst_id = context_.definitions_required()[i];
+  // NOLINTNEXTLINE(modernize-loop-convert)
+  for (size_t i = 0; i != context_.definitions_required_by_decl().size(); ++i) {
+    SemIR::InstId decl_inst_id = context_.definitions_required_by_decl()[i];
     SemIR::Inst decl_inst = context_.insts().Get(decl_inst_id);
     CARBON_KIND_SWITCH(context_.insts().Get(decl_inst_id)) {
       case CARBON_KIND(SemIR::ClassDecl class_decl): {
@@ -474,33 +478,32 @@ auto CheckUnit::CheckRequiredDefinitions() -> void {
         // https://github.com/carbon-language/carbon-lang/issues/4071.
         CARBON_FATAL("TODO: Support interfaces in DiagnoseMissingDefinitions");
       }
-      case CARBON_KIND(SemIR::SpecificFunction specific_function): {
-        // TODO: Track a location for the use. In general we may want to track a
-        // list of enclosing locations if this was used from a generic.
-        SemIRLoc use_loc = decl_inst_id;
-        if (!ResolveSpecificDefinition(context_, use_loc,
-                                       specific_function.specific_id)) {
-          CARBON_DIAGNOSTIC(MissingGenericFunctionDefinition, Error,
-                            "use of undefined generic function");
-          CARBON_DIAGNOSTIC(MissingGenericFunctionDefinitionHere, Note,
-                            "generic function declared here");
-          auto generic_decl_id =
-              context_.generics()
-                  .Get(context_.specifics()
-                           .Get(specific_function.specific_id)
-                           .generic_id)
-                  .decl_id;
-          emitter_.Build(decl_inst_id, MissingGenericFunctionDefinition)
-              .Note(generic_decl_id, MissingGenericFunctionDefinitionHere)
-              .Emit();
-        }
-        break;
-      }
       default: {
         CARBON_FATAL("Unexpected inst in definitions_required: {0}", decl_inst);
       }
     }
   }
+
+  // Note that more required definitions can be added during this loop.
+  // NOLINTNEXTLINE(modernize-loop-convert)
+  for (size_t i = 0; i != context_.definitions_required_by_use().size(); ++i) {
+    // This is using the location for the use. We could track the
+    // list of enclosing locations if this was used from a generic.
+    auto [loc, specific_id] = context_.definitions_required_by_use()[i];
+    if (!ResolveSpecificDefinition(context_, loc, specific_id)) {
+      CARBON_DIAGNOSTIC(MissingGenericFunctionDefinition, Error,
+                        "use of undefined generic function");
+      CARBON_DIAGNOSTIC(MissingGenericFunctionDefinitionHere, Note,
+                        "generic function declared here");
+      auto generic_decl_id =
+          context_.generics()
+              .Get(context_.specifics().Get(specific_id).generic_id)
+              .decl_id;
+      emitter_.Build(loc, MissingGenericFunctionDefinition)
+          .Note(generic_decl_id, MissingGenericFunctionDefinitionHere)
+          .Emit();
+    }
+  }
 }
 
 auto CheckUnit::FinishRun() -> void {

+ 2 - 2
toolchain/check/check_unit.h

@@ -162,8 +162,8 @@ class CheckUnit {
 
   // Checks that each required definition is available. If the definition can be
   // generated by resolving a specific, does so, otherwise emits a diagnostic
-  // for each declaration in context.definitions_required() that doesn't have a
-  // definition.
+  // for each declaration in context.definitions_required_by_decl() and
+  // context.definitions_required_by_use that doesn't have a definition.
   auto CheckRequiredDefinitions() -> void;
 
   // Does work after processing the parse tree, such as finishing the IR and

+ 14 - 3
toolchain/check/context.h

@@ -147,8 +147,13 @@ class Context {
     return import_ir_constant_values_;
   }
 
-  auto definitions_required() -> llvm::SmallVector<SemIR::InstId>& {
-    return definitions_required_;
+  auto definitions_required_by_decl() -> llvm::SmallVector<SemIR::InstId>& {
+    return definitions_required_by_decl_;
+  }
+
+  auto definitions_required_by_use()
+      -> llvm::SmallVector<std::pair<SemIRLoc, SemIR::SpecificId>>& {
+    return definitions_required_by_use_;
   }
 
   auto global_init() -> GlobalInit& { return global_init_; }
@@ -343,7 +348,13 @@ class Context {
 
   // Declaration instructions of entities that should have definitions by the
   // end of the current source file.
-  llvm::SmallVector<SemIR::InstId> definitions_required_;
+  llvm::SmallVector<SemIR::InstId> definitions_required_by_decl_;
+
+  // Entities that should have definitions by the end of the current source
+  // file, because of a generic was used a concrete specific. This is currently
+  // only tracking specific functions that should have a definition emitted.
+  llvm::SmallVector<std::pair<SemIRLoc, SemIR::SpecificId>>
+      definitions_required_by_use_;
 
   // State for global initialization.
   GlobalInit global_init_;

+ 14 - 1
toolchain/check/eval_inst.cpp

@@ -11,6 +11,7 @@
 #include "toolchain/check/generic.h"
 #include "toolchain/check/impl_lookup.h"
 #include "toolchain/check/import_ref.h"
+#include "toolchain/check/inst.h"
 #include "toolchain/check/type.h"
 #include "toolchain/check/type_completion.h"
 #include "toolchain/sem_ir/ids.h"
@@ -393,14 +394,26 @@ auto EvalConstantInst(Context& context, SemIRLoc loc,
   args.append(interface_fn_args.end() - remaining_params,
               interface_fn_args.end());
   auto specific_id = MakeSpecific(context, loc, generic_id, args);
+  context.definitions_required_by_use().push_back({loc, specific_id});
 
-  // TODO: Add the new `SpecificFunction` to definitions_required.
   return ConstantEvalResult::NewSamePhase(
       SemIR::SpecificFunction{.type_id = inst.type_id,
                               .callee_id = inst.callee_id,
                               .specific_id = specific_id});
 }
 
+auto EvalConstantInst(Context& context, SemIRLoc loc,
+                      SemIR::SpecificFunction inst) -> ConstantEvalResult {
+  if (!SemIR::GetCalleeFunction(context.sem_ir(), inst.callee_id)
+           .self_type_id.has_value()) {
+    // This is not an associated function. Those will be required to be defined
+    // as part of checking that the impl is complete.
+    context.definitions_required_by_use().push_back({loc, inst.specific_id});
+  }
+  // Create new constant for a specific function.
+  return ConstantEvalResult::NewSamePhase(inst);
+}
+
 auto EvalConstantInst(Context& context, SemIRLoc /*loc*/,
                       SemIR::SpliceBlock inst) -> ConstantEvalResult {
   // SpliceBlock evaluates to the result value that is (typically) within the

+ 1 - 1
toolchain/check/handle_class.cpp

@@ -275,7 +275,7 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,
   }
 
   if (!is_definition && context.sem_ir().is_impl() && !is_extern) {
-    context.definitions_required().push_back(class_decl_id);
+    context.definitions_required_by_decl().push_back(class_decl_id);
   }
 
   return {class_decl.class_id, class_decl_id};

+ 1 - 1
toolchain/check/handle_function.cpp

@@ -531,7 +531,7 @@ static auto BuildFunctionDecl(Context& context,
                         function_info);
 
   if (!is_definition && context.sem_ir().is_impl() && !is_extern) {
-    context.definitions_required().push_back(decl_id);
+    context.definitions_required_by_decl().push_back(decl_id);
   }
 
   return {function_decl.function_id, decl_id};

+ 1 - 1
toolchain/check/handle_impl.cpp

@@ -506,7 +506,7 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
   if (!is_definition && !invalid_redeclaration &&
       context.impls().Get(impl_decl.impl_id).witness_id !=
           SemIR::ErrorInst::SingletonInstId) {
-    context.definitions_required().push_back(impl_decl_id);
+    context.definitions_required_by_decl().push_back(impl_decl_id);
   }
 
   return {impl_decl.impl_id, impl_decl_id};

+ 2 - 0
toolchain/check/testdata/facet/min_prelude/convert_facet_value_to_itself.carbon

@@ -172,6 +172,8 @@ fn F() {
 // CHECK:STDOUT: specific @FeedAnimal(constants.%Animal.facet) {
 // CHECK:STDOUT:   %T.loc16_15.2 => constants.%Animal.facet
 // CHECK:STDOUT:   %T.patt.loc16_15.2 => constants.%T.patt
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- include_files/convert.carbon

+ 4 - 0
toolchain/check/testdata/facet/min_prelude/convert_facet_value_value_to_generic_facet_value_value.carbon

@@ -532,6 +532,10 @@ fn F() {
 // CHECK:STDOUT:   %T.loc30_24.2 => constants.%Eats.facet.fa6
 // CHECK:STDOUT:   %T.patt.loc30_24.2 => constants.%T.patt.1ac
 // CHECK:STDOUT:   %T.as_type.loc30_43.2 => constants.%Goat
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %require_complete.loc30_41 => constants.%complete_type.357
+// CHECK:STDOUT:   %require_complete.loc30_50 => constants.%complete_type.357
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- include_files/convert.carbon

+ 3 - 0
toolchain/check/testdata/facet/min_prelude/convert_facet_value_value_to_itself.carbon

@@ -213,6 +213,9 @@ fn F() {
 // CHECK:STDOUT:   %T.loc16_15.2 => constants.%Animal.facet
 // CHECK:STDOUT:   %T.patt.loc16_15.2 => constants.%T.patt
 // CHECK:STDOUT:   %T.as_type.loc16_30.2 => constants.%Goat
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %require_complete => constants.%complete_type
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- include_files/convert.carbon

+ 0 - 2
toolchain/check/testdata/facet/min_prelude/fail_convert_class_type_to_generic_facet_value.carbon

@@ -265,8 +265,6 @@ fn G() {
 // CHECK:STDOUT:   %Generic.type.loc26_45.2 => constants.%Generic.type.c3b
 // CHECK:STDOUT:   %U.loc26_32.2 => <error>
 // CHECK:STDOUT:   %U.patt.loc26_32.2 => constants.%U.patt.19b
-// CHECK:STDOUT:
-// CHECK:STDOUT: !definition:
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- include_files/convert.carbon

+ 0 - 3
toolchain/check/testdata/facet/min_prelude/fail_deduction_uses_runtime_type_conversion.carbon

@@ -384,9 +384,6 @@ fn G(holds_to: HoldsType((RuntimeConvertTo, ))) {
 // CHECK:STDOUT:   %A.loc26_20.2 => <error>
 // CHECK:STDOUT:   %A.patt.loc26_20.2 => constants.%A.patt.78b
 // CHECK:STDOUT:   %HoldsType.loc26_43.2 => constants.%HoldsType.066
-// CHECK:STDOUT:
-// CHECK:STDOUT: !definition:
-// CHECK:STDOUT:   %require_complete => constants.%complete_type
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- include_files/convert.carbon

+ 7 - 0
toolchain/check/testdata/generic/call_basic_depth.carbon

@@ -361,10 +361,17 @@ fn M() {
 // CHECK:STDOUT: specific @H(constants.%i32) {
 // CHECK:STDOUT:   %T.loc19_6.2 => constants.%i32
 // CHECK:STDOUT:   %T.patt.loc19_6.2 => constants.%T.patt
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %require_complete => constants.%complete_type.f8a
+// CHECK:STDOUT:   %F.specific_fn.loc21_3.2 => constants.%F.specific_fn.501
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @Cfn(constants.%i32) {
 // CHECK:STDOUT:   %T.loc12_22.1 => constants.%i32
 // CHECK:STDOUT:   %T.patt.loc12_22.2 => constants.%T.patt
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %require_complete => constants.%complete_type.f8a
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 2 - 0
toolchain/check/testdata/where_expr/equal_rewrite.carbon

@@ -621,6 +621,8 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT: specific @OneRewrite(constants.%I) {
 // CHECK:STDOUT:   %G.loc8_15.2 => constants.%I
 // CHECK:STDOUT:   %G.patt.loc8_15.2 => constants.%G.patt
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- rewrites_reordered.carbon

+ 21 - 22
toolchain/lower/testdata/function/generic/call_basic_depth.carbon

@@ -12,10 +12,7 @@ fn F[T:! type](x: T) {
 }
 
 fn H[T:! type](x: T) -> T {
-  // This call crashes because check/eval does not emit a definition for H.
-  // In G below, the call to H can be typechecked without needing a definition.
-  // TODO: this needs to be resolved in check/eval.
-  // F(x);
+  F(x);
   return x;
 }
 
@@ -45,10 +42,10 @@ fn M() {
 // CHECK:STDOUT:   call void @llvm.lifetime.start.p0(i64 4, ptr %n.var), !dbg !7
 // CHECK:STDOUT:   store i32 0, ptr %n.var, align 4, !dbg !7
 // CHECK:STDOUT:   call void @llvm.lifetime.start.p0(i64 4, ptr %m.var), !dbg !7
-// CHECK:STDOUT:   %.loc32 = load i32, ptr %n.var, align 4, !dbg !8
-// CHECK:STDOUT:   call void @_CF.Main.b88d1103f417c6d4(i32 %.loc32), !dbg !9
-// CHECK:STDOUT:   %.loc33 = load i32, ptr %n.var, align 4, !dbg !10
-// CHECK:STDOUT:   %G.call = call i32 @_CG.Main.b88d1103f417c6d4(i32 %.loc33), !dbg !11
+// CHECK:STDOUT:   %.loc29 = load i32, ptr %n.var, align 4, !dbg !8
+// CHECK:STDOUT:   call void @_CF.Main.b88d1103f417c6d4(i32 %.loc29), !dbg !9
+// CHECK:STDOUT:   %.loc30 = load i32, ptr %n.var, align 4, !dbg !10
+// CHECK:STDOUT:   %G.call = call i32 @_CG.Main.b88d1103f417c6d4(i32 %.loc30), !dbg !11
 // CHECK:STDOUT:   store i32 %G.call, ptr %m.var, align 4, !dbg !12
 // CHECK:STDOUT:   ret void, !dbg !13
 // CHECK:STDOUT: }
@@ -70,7 +67,8 @@ fn M() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: define i32 @_CH.Main.b88d1103f417c6d4(i32 %x) !dbg !20 {
 // CHECK:STDOUT: entry:
-// CHECK:STDOUT:   ret i32 %x, !dbg !21
+// CHECK:STDOUT:   call void @_CF.Main.b88d1103f417c6d4(i32 %x), !dbg !21
+// CHECK:STDOUT:   ret i32 %x, !dbg !22
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: ; uselistorder directives
@@ -85,21 +83,22 @@ fn M() {
 // CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
 // CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
 // CHECK:STDOUT: !3 = !DIFile(filename: "call_basic_depth.carbon", directory: "")
-// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "M", linkageName: "_CM.Main", scope: null, file: !3, line: 28, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "M", linkageName: "_CM.Main", scope: null, file: !3, line: 25, type: !5, spFlags: DISPFlagDefinition, unit: !2)
 // CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
 // CHECK:STDOUT: !6 = !{}
-// CHECK:STDOUT: !7 = !DILocation(line: 29, column: 3, scope: !4)
-// CHECK:STDOUT: !8 = !DILocation(line: 32, column: 5, scope: !4)
-// CHECK:STDOUT: !9 = !DILocation(line: 32, column: 3, scope: !4)
-// CHECK:STDOUT: !10 = !DILocation(line: 33, column: 9, scope: !4)
-// CHECK:STDOUT: !11 = !DILocation(line: 33, column: 7, scope: !4)
-// CHECK:STDOUT: !12 = !DILocation(line: 33, column: 3, scope: !4)
-// CHECK:STDOUT: !13 = !DILocation(line: 28, column: 1, scope: !4)
+// CHECK:STDOUT: !7 = !DILocation(line: 26, column: 3, scope: !4)
+// CHECK:STDOUT: !8 = !DILocation(line: 29, column: 5, scope: !4)
+// CHECK:STDOUT: !9 = !DILocation(line: 29, column: 3, scope: !4)
+// CHECK:STDOUT: !10 = !DILocation(line: 30, column: 9, scope: !4)
+// CHECK:STDOUT: !11 = !DILocation(line: 30, column: 7, scope: !4)
+// CHECK:STDOUT: !12 = !DILocation(line: 30, column: 3, scope: !4)
+// CHECK:STDOUT: !13 = !DILocation(line: 25, column: 1, scope: !4)
 // CHECK:STDOUT: !14 = distinct !DISubprogram(name: "F", linkageName: "_CF.Main.b88d1103f417c6d4", scope: null, file: !3, line: 11, type: !5, spFlags: DISPFlagDefinition, unit: !2)
 // CHECK:STDOUT: !15 = !DILocation(line: 11, column: 1, scope: !14)
-// CHECK:STDOUT: !16 = distinct !DISubprogram(name: "G", linkageName: "_CG.Main.b88d1103f417c6d4", scope: null, file: !3, line: 22, type: !5, spFlags: DISPFlagDefinition, unit: !2)
-// CHECK:STDOUT: !17 = !DILocation(line: 23, column: 3, scope: !16)
-// CHECK:STDOUT: !18 = !DILocation(line: 24, column: 3, scope: !16)
-// CHECK:STDOUT: !19 = !DILocation(line: 25, column: 3, scope: !16)
+// CHECK:STDOUT: !16 = distinct !DISubprogram(name: "G", linkageName: "_CG.Main.b88d1103f417c6d4", scope: null, file: !3, line: 19, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !17 = !DILocation(line: 20, column: 3, scope: !16)
+// CHECK:STDOUT: !18 = !DILocation(line: 21, column: 3, scope: !16)
+// CHECK:STDOUT: !19 = !DILocation(line: 22, column: 3, scope: !16)
 // CHECK:STDOUT: !20 = distinct !DISubprogram(name: "H", linkageName: "_CH.Main.b88d1103f417c6d4", scope: null, file: !3, line: 14, type: !5, spFlags: DISPFlagDefinition, unit: !2)
-// CHECK:STDOUT: !21 = !DILocation(line: 19, column: 3, scope: !20)
+// CHECK:STDOUT: !21 = !DILocation(line: 15, column: 3, scope: !20)
+// CHECK:STDOUT: !22 = !DILocation(line: 16, column: 3, scope: !20)

+ 1 - 1
toolchain/sem_ir/typed_insts.h

@@ -1420,7 +1420,7 @@ struct SpecificConstant {
 struct SpecificFunction {
   static constexpr auto Kind = InstKind::SpecificFunction.Define<Parse::NodeId>(
       {.ir_name = "specific_function",
-       .constant_kind = InstConstantKind::WheneverPossible});
+       .constant_kind = InstConstantKind::Conditional});
 
   // Always the builtin SpecificFunctionType.
   TypeId type_id;