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

Handle some more errors in interfaces without crashing. (#6155)

This fixes and tests two crashes related to what I was observing [on
#toolchain](https://discord.com/channels/655572317891461132/655578254970716160/1422723976483831848).

The approach to Cpp imports taken in #6086 is problematic because it
returns before the work stack is completed, and doesn't store the
resulting constants. This fixes the approach taken in that PR.
Jon Ross-Perkins 7 месяцев назад
Родитель
Сommit
81c2b3be1a

+ 10 - 4
toolchain/check/import_ref.cpp

@@ -450,9 +450,6 @@ class ImportRefResolver : public ImportContext {
 
       // Step 1: check for a constant value.
       auto existing = FindResolvedConstId(work.inst_id);
-      if (existing.const_id == SemIR::ErrorInst::ConstantId) {
-        return existing.const_id;
-      }
       if (existing.const_id.has_value() && !work.retry_with_constant_value) {
         work_stack_.pop_back();
         continue;
@@ -592,7 +589,11 @@ class ImportRefResolver : public ImportContext {
       if (ir_inst.ir_id() == SemIR::ImportIRId::Cpp) {
         local_context().TODO(SemIR::LocId::None,
                              "Unsupported: Importing C++ indirectly");
-        return {.const_id = SemIR::ErrorInst::ConstantId};
+        SetResolvedConstId(inst_id, result.indirect_insts,
+                           SemIR::ErrorInst::ConstantId);
+        result.const_id = SemIR::ErrorInst::ConstantId;
+        result.indirect_insts.clear();
+        return result;
       }
 
       const auto* prev_ir = cursor_ir;
@@ -1794,6 +1795,11 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
   auto class_const_id = GetLocalConstantId(
       resolver,
       resolver.import_classes().Get(inst.class_id).first_owning_decl_id);
+  if (class_const_id == SemIR::ErrorInst::ConstantId) {
+    // TODO: It should be possible to remove this once C++ imports work.
+    return ResolveResult::Done(class_const_id);
+  }
+
   auto specific_data = GetLocalSpecificData(resolver, inst.specific_id);
   if (resolver.HasNewWork()) {
     return ResolveResult::Retry();

+ 12 - 6
toolchain/check/interface.cpp

@@ -181,8 +181,13 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id,
                                         SemIR::TypeId self_type_id,
                                         SemIR::InstId self_witness_id)
     -> SemIR::TypeId {
-  auto decl =
-      context.insts().Get(context.constant_values().GetConstantInstId(decl_id));
+  auto decl_constant_inst_id =
+      context.constant_values().GetConstantInstId(decl_id);
+  if (decl_constant_inst_id == SemIR::ErrorInst::InstId) {
+    return SemIR::ErrorInst::TypeId;
+  }
+
+  auto decl = context.insts().Get(decl_constant_inst_id);
   if (auto assoc_const = decl.TryAs<SemIR::AssociatedConstantDecl>()) {
     // Form a specific for the associated constant, and grab the type from
     // there.
@@ -195,8 +200,9 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id,
     auto const_specific_id = MakeSpecific(context, loc_id, generic_id, arg_ids);
     return SemIR::GetTypeOfInstInSpecific(context.sem_ir(), const_specific_id,
                                           decl_id);
-  } else if (auto fn = context.types().TryGetAs<SemIR::FunctionType>(
-                 decl.type_id())) {
+  }
+
+  if (auto fn = context.types().TryGetAs<SemIR::FunctionType>(decl.type_id())) {
     // Form the type of the function within the interface, and attach the `Self`
     // type.
     auto interface_fn_type_id = SemIR::GetTypeOfInstInSpecific(
@@ -208,9 +214,9 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id,
     return GetFunctionTypeWithSelfType(
         context, context.types().GetInstId(interface_fn_type_id),
         self_facet_id);
-  } else {
-    CARBON_FATAL("Unexpected kind for associated constant {0}", decl);
   }
+
+  CARBON_FATAL("Unexpected kind for associated constant {0}", decl);
 }
 
 }  // namespace Carbon::Check

+ 124 - 0
toolchain/check/testdata/impl/using_invalid_interface.carbon

@@ -0,0 +1,124 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/convert.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/using_invalid_interface.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/using_invalid_interface.carbon
+
+// --- fail_invalid_interface.carbon
+library "[[@TEST_NAME]]";
+
+interface I {}
+
+class ArgToI(T:! I) {}
+
+interface IMisuse {
+  let Arg:! type;
+  // CHECK:STDERR: fail_invalid_interface.carbon:[[@LINE+7]]:26: error: cannot convert type `Arg` into type implementing `I` [ConversionFailureTypeToFacet]
+  // CHECK:STDERR:   fn Op[self: Self]() -> ArgToI(Arg);
+  // CHECK:STDERR:                          ^~~~~~~~~~~
+  // CHECK:STDERR: fail_invalid_interface.carbon:[[@LINE-7]]:14: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+  // CHECK:STDERR: class ArgToI(T:! I) {}
+  // CHECK:STDERR:              ^
+  // CHECK:STDERR:
+  fn Op[self: Self]() -> ArgToI(Arg);
+}
+
+// --- fail_misusing_invalid_interface.carbon
+library "[[@TEST_NAME]]";
+// This import boundary is significant.
+import library "invalid_interface";
+
+class C {
+  // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+4]]:32: error: name `T` not found [NameNotFound]
+  // CHECK:STDERR:   impl as IMisuse where .Arg = T {
+  // CHECK:STDERR:                                ^
+  // CHECK:STDERR:
+  impl as IMisuse where .Arg = T {
+    // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+4]]:35: error: name `T` not found [NameNotFound]
+    // CHECK:STDERR:     fn Op[self: Self]() -> ArgToI(T) {
+    // CHECK:STDERR:                                   ^
+    // CHECK:STDERR:
+    fn Op[self: Self]() -> ArgToI(T) {
+      return {};
+    }
+  }
+}
+
+// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+8]]:21: error: missing object argument in method call [MissingObjectInMethodCall]
+// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
+// CHECK:STDERR:                     ^~~~~~
+// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE-21]]:1: in import [InImport]
+// CHECK:STDERR: fail_invalid_interface.carbon:16:3: note: calling function declared here [InCallToFunction]
+// CHECK:STDERR:   fn Op[self: Self]() -> ArgToI(Arg);
+// CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+fn F(T:! IMisuse) { T.Op(); }
+
+fn G() {
+  var c: C;
+  // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+10]]:3: error: cannot implicitly convert non-type value of type `C` into type implementing `IMisuse` [ConversionFailureNonTypeToFacet]
+  // CHECK:STDERR:   F(c);
+  // CHECK:STDERR:   ^~~~
+  // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+7]]:3: note: type `C` does not implement interface `Core.ImplicitAs(IMisuse)` [MissingImplInMemberAccessNote]
+  // CHECK:STDERR:   F(c);
+  // CHECK:STDERR:   ^~~~
+  // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE-10]]:6: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+  // CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
+  // CHECK:STDERR:      ^
+  // CHECK:STDERR:
+  F(c);
+}
+
+// --- fail_using_invalid_interface.carbon
+library "[[@TEST_NAME]]";
+// This import boundary is significant.
+import library "invalid_interface";
+
+class ArgC {}
+
+class C {
+  impl as IMisuse where .Arg = ArgC {
+    // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+8]]:28: error: cannot convert type `ArgC` into type implementing `I` [ConversionFailureTypeToFacet]
+    // CHECK:STDERR:     fn Op[self: Self]() -> ArgToI(ArgC) {
+    // CHECK:STDERR:                            ^~~~~~~~~~~~
+    // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-9]]:1: in import [InImport]
+    // CHECK:STDERR: fail_invalid_interface.carbon:5:14: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+    // CHECK:STDERR: class ArgToI(T:! I) {}
+    // CHECK:STDERR:              ^
+    // CHECK:STDERR:
+    fn Op[self: Self]() -> ArgToI(ArgC) {
+      return {};
+    }
+  }
+}
+
+// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+8]]:21: error: missing object argument in method call [MissingObjectInMethodCall]
+// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
+// CHECK:STDERR:                     ^~~~~~
+// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-23]]:1: in import [InImport]
+// CHECK:STDERR: fail_invalid_interface.carbon:16:3: note: calling function declared here [InCallToFunction]
+// CHECK:STDERR:   fn Op[self: Self]() -> ArgToI(Arg);
+// CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+fn F(T:! IMisuse) { T.Op(); }
+
+fn G() {
+  var c: C;
+  // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+10]]:3: error: cannot implicitly convert non-type value of type `C` into type implementing `IMisuse` [ConversionFailureNonTypeToFacet]
+  // CHECK:STDERR:   F(c);
+  // CHECK:STDERR:   ^~~~
+  // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+7]]:3: note: type `C` does not implement interface `Core.ImplicitAs(IMisuse)` [MissingImplInMemberAccessNote]
+  // CHECK:STDERR:   F(c);
+  // CHECK:STDERR:   ^~~~
+  // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-10]]:6: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+  // CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
+  // CHECK:STDERR:      ^
+  // CHECK:STDERR:
+  F(c);
+}