From 2e61685658f87a1a685b1bff49bdd5b00ab49509 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Tue, 21 Jul 2026 14:35:20 -0400 Subject: [PATCH] Remove duplicate check for exporting a specific class (#7541) The check for a specific in `TryMapClassType` is unnecessary; immediately after it calls `ExportClassToCpp`, which has the same check. The latter also has a `context.TODO`, which provides a clearer error. Also improved the `LocId` in `ExportClassToCpp` to use the location of the first decl rather than the empty location of the class type. This is the same fix as https://github.com/carbon-language/carbon-lang/pull/7533, just applied a little more broadly. This makes the `context.TODO` above point at the class rather than the start of the source file. --- toolchain/check/cpp/export.cpp | 12 ++++++------ toolchain/check/cpp/export.h | 4 ++-- toolchain/check/cpp/type_mapping.cpp | 15 ++++----------- .../interop/cpp/class/export/class.carbon | 6 +++++- .../interop/cpp/function/export/generic.carbon | 6 +++++- .../cpp/function/import/param_unsupported.carbon | 6 +++++- 6 files changed, 27 insertions(+), 22 deletions(-) diff --git a/toolchain/check/cpp/export.cpp b/toolchain/check/cpp/export.cpp index 2ac0d99c1824..cbe8889bb486 100644 --- a/toolchain/check/cpp/export.cpp +++ b/toolchain/check/cpp/export.cpp @@ -130,18 +130,19 @@ auto ExportNameScopeToCpp(Context& context, SemIR::LocId loc_id, return decl_context; } -auto ExportClassToCpp(Context& context, SemIR::LocId loc_id, - SemIR::ClassType class_type) -> clang::TagDecl* { +auto ExportClassToCpp(Context& context, SemIR::ClassType class_type) + -> clang::TagDecl* { // TODO: A lot of logic in this function is shared with ExportNameScopeToCpp. // This should be refactored. + const auto& class_info = context.classes().Get(class_type.class_id); + SemIR::LocId loc_id(class_info.first_decl_id()); + if (class_type.specific_id.has_value()) { context.TODO(loc_id, "interop with specific class"); return nullptr; } - const auto& class_info = context.classes().Get(class_type.class_id); - // If this class was produced by importing a C++ declaration or has // already been exported to C++, return the corresponding Clang declaration. // That could either be a CXXRecordDecl or an EnumDecl. @@ -156,8 +157,7 @@ auto ExportClassToCpp(Context& context, SemIR::LocId loc_id, auto* decl_context = ExportNameScopeToCpp(context, loc_id, class_info.parent_scope_id); - auto clang_loc = - GetCppLocation(context, SemIR::LocId(class_info.first_decl_id())); + auto clang_loc = GetCppLocation(context, loc_id); auto* record_decl = clang::CXXRecordDecl::Create( context.ast_context(), clang::TagTypeKind::Class, decl_context, clang_loc, clang_loc, identifier_info); diff --git a/toolchain/check/cpp/export.h b/toolchain/check/cpp/export.h index 9b505938f74e..3c90a58faf2c 100644 --- a/toolchain/check/cpp/export.h +++ b/toolchain/check/cpp/export.h @@ -33,8 +33,8 @@ auto ExportNameScopeToCpp(Context& context, SemIR::LocId loc_id, // If the class has already been exported, returns the existing C++ class. // Otherwise, creates a new C++ class and returns it. Returns nullptr if the // class could not be exported and an error was diagnosed. -auto ExportClassToCpp(Context& context, SemIR::LocId loc_id, - SemIR::ClassType class_type) -> clang::TagDecl*; +auto ExportClassToCpp(Context& context, SemIR::ClassType class_type) + -> clang::TagDecl*; // Export all `SemIR::FieldDecl`s in the class body as `clang::FieldDecl`s. auto ExportAllFieldsToCpp(Context& context, SemIR::Class& class_info) -> void; diff --git a/toolchain/check/cpp/type_mapping.cpp b/toolchain/check/cpp/type_mapping.cpp index d847fa073653..05da556c5789 100644 --- a/toolchain/check/cpp/type_mapping.cpp +++ b/toolchain/check/cpp/type_mapping.cpp @@ -140,8 +140,8 @@ static auto VerifyIntegerTypeWidth(Context& context, clang::QualType type, // Maps a Carbon class type to a C++ type. Returns a null `QualType` if the // type is not supported. -static auto TryMapClassType(Context& context, SemIR::TypeInstId class_inst_id, - SemIR::ClassType class_type) -> TryMapTypeResult { +static auto TryMapClassType(Context& context, SemIR::ClassType class_type) + -> TryMapTypeResult { clang::ASTContext& ast_context = context.ast_context(); // If the class represents a Carbon type literal, map it to the corresponding @@ -222,14 +222,8 @@ static auto TryMapClassType(Context& context, SemIR::TypeInstId class_inst_id, } } - // TODO: We cannot yet map specific classes. - if (class_type.specific_id.has_value()) { - return clang::QualType(); - } - // Otherwise, find the existing C++ declaration or create a new one. - auto* tag_decl = - ExportClassToCpp(context, SemIR::LocId(class_inst_id), class_type); + auto* tag_decl = ExportClassToCpp(context, class_type); if (!tag_decl) { return clang::QualType(); } @@ -265,8 +259,7 @@ static auto TryMapType(Context& context, SemIR::TypeId type_id) return context.ast_context().CharTy; } case CARBON_KIND(SemIR::ClassType class_type): { - return TryMapClassType(context, context.types().GetTypeInstId(type_id), - class_type); + return TryMapClassType(context, class_type); } case CARBON_KIND(SemIR::ConstType const_type): { return WrappedType{ diff --git a/toolchain/check/testdata/interop/cpp/class/export/class.carbon b/toolchain/check/testdata/interop/cpp/class/export/class.carbon index a3a88b6135b3..22c2d95195aa 100644 --- a/toolchain/check/testdata/interop/cpp/class/export/class.carbon +++ b/toolchain/check/testdata/interop/cpp/class/export/class.carbon @@ -81,6 +81,10 @@ library "[[@TEST_NAME]]"; import Cpp; +// CHECK:STDERR: fail_todo_monomorphization_failure.carbon:[[@LINE+4]]:1: error: semantics TODO: `interop with specific class` [SemanticsTodo] +// CHECK:STDERR: class B(N: i32) { +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~ +// CHECK:STDERR: class B(N: i32) { var a: array(i8, N); } @@ -95,7 +99,7 @@ inline Cpp ''' // CHECK:STDERR: ^ // CHECK:STDERR: // CHECK:STDERR: fail_todo_monomorphization_failure.carbon:[[@LINE+4]]:9: error: no type named 'T' in namespace 'Carbon' [CppInteropParseError] -// CHECK:STDERR: 22 | Carbon::T t; +// CHECK:STDERR: 26 | Carbon::T t; // CHECK:STDERR: | ~~~~~~~~^ // CHECK:STDERR: Carbon::T t; diff --git a/toolchain/check/testdata/interop/cpp/function/export/generic.carbon b/toolchain/check/testdata/interop/cpp/function/export/generic.carbon index 3503b1b2ab79..7a5b62601af9 100644 --- a/toolchain/check/testdata/interop/cpp/function/export/generic.carbon +++ b/toolchain/check/testdata/interop/cpp/function/export/generic.carbon @@ -90,6 +90,10 @@ void G() { library "[[@TEST_NAME]]"; import Cpp; +// CHECK:STDERR: fail_todo_enclosing_generic.carbon:[[@LINE+4]]:1: error: semantics TODO: `interop with specific class` [SemanticsTodo] +// CHECK:STDERR: class A(T: type) { +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: class A(T: type) { fn F[U: type](x: T, y: U); } @@ -105,7 +109,7 @@ void f() { // CHECK:STDERR: ^ // CHECK:STDERR: // CHECK:STDERR: fail_todo_enclosing_generic.carbon:[[@LINE+4]]:11: error: no type named 'B' in namespace 'Carbon' [CppInteropParseError] - // CHECK:STDERR: 22 | Carbon::B b; + // CHECK:STDERR: 26 | Carbon::B b; // CHECK:STDERR: | ~~~~~~~~^ // CHECK:STDERR: Carbon::B b; diff --git a/toolchain/check/testdata/interop/cpp/function/import/param_unsupported.carbon b/toolchain/check/testdata/interop/cpp/function/import/param_unsupported.carbon index c2e5a4d6bc08..1b36196fedf2 100644 --- a/toolchain/check/testdata/interop/cpp/function/import/param_unsupported.carbon +++ b/toolchain/check/testdata/interop/cpp/function/import/param_unsupported.carbon @@ -74,10 +74,14 @@ import Cpp library "int_param.h"; // This doesn't have a direct type mapping in `getIntTypeForBitwidth`. fn F(x: i512) { - // CHECK:STDERR: fail_large_int.carbon:[[@LINE+4]]:11: error: call argument of type `i512` is not supported [CppCallArgTypeNotSupported] + // CHECK:STDERR: fail_large_int.carbon:[[@LINE+8]]:11: error: call argument of type `i512` is not supported [CppCallArgTypeNotSupported] // CHECK:STDERR: Cpp.foo(x); // CHECK:STDERR: ^ // CHECK:STDERR: + // CHECK:STDERR: min_prelude/parts/int.carbon:10:1: error: semantics TODO: `interop with specific class` [SemanticsTodo] + // CHECK:STDERR: class Int(N: IntLiteral) { + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: Cpp.foo(x); }