diff --git a/toolchain/check/cpp/export.cpp b/toolchain/check/cpp/export.cpp index 6534fadc231b..e0fdec4e3a66 100644 --- a/toolchain/check/cpp/export.cpp +++ b/toolchain/check/cpp/export.cpp @@ -4,6 +4,8 @@ #include "toolchain/check/cpp/export.h" +#include + #include "llvm/Support/Casting.h" #include "toolchain/check/cpp/import.h" #include "toolchain/check/cpp/location.h" @@ -330,6 +332,11 @@ auto CalculateCppFieldOffsets( namespace { struct FunctionInfo { struct Param { + Param(Context& context, SemIR::InstId param_inst_id) + : type_id(ExtractScrutineeType( + context.sem_ir(), context.insts().Get(param_inst_id).type_id())), + is_ref(context.insts().Is(param_inst_id)) {} + // Type of the parameter's scrutinee. SemIR::TypeId type_id; @@ -346,15 +353,13 @@ struct FunctionInfo { auto function_params = context.inst_blocks().Get(function.call_param_patterns_id); - // Get the function's `self` parameter type, if present. + // Get the function's `self` parameter, if present. if (function.call_param_ranges.implicit_size() > 0) { CARBON_CHECK(function.call_param_ranges.implicit_size() == 1); auto param_inst_id = function_params[function.call_param_ranges.implicit_begin().index]; - auto scrutinee_type_id = ExtractScrutineeType( - context.sem_ir(), context.insts().Get(param_inst_id).type_id()); - self_type_id = scrutinee_type_id; + self_param = Param(context, param_inst_id); } // Get the function's explicit parameters. @@ -363,25 +368,27 @@ struct FunctionInfo { function_params = function_params.drop_back(function.call_param_ranges.return_size()); for (auto param_inst_id : function_params) { - explicit_params.push_back( - {.type_id = ExtractScrutineeType( - context.sem_ir(), context.insts().Get(param_inst_id).type_id()), - .is_ref = - context.insts().Is(param_inst_id)}); + explicit_params.push_back(Param(context, param_inst_id)); } } // Get the `StorageClass` to use for `CXXMethodDecl`s. auto GetStorageClass() const -> clang::StorageClass { - if (has_self()) { + if (self_param) { return clang::SC_None; } else { return clang::SC_Static; } } - // Whether the function has a `self` parameter. - auto has_self() const -> bool { return self_type_id != SemIR::TypeId::None; } + // Get the `self` param type, or `None` if the function does not have + // a `self` param. + auto GetSelfTypeId() const -> SemIR::TypeId { + if (self_param) { + return self_param->type_id; + } + return SemIR::TypeId::None; + } SemIR::FunctionId function_id; const SemIR::Function& function; @@ -395,9 +402,9 @@ struct FunctionInfo { // and whether the parameter is a reference. llvm::SmallVector explicit_params; - // Type of the function's `self` parameter, or `None` if the function - // is not a method. - SemIR::TypeId self_type_id = SemIR::TypeId::None; + // For methods, the type of `self` and whether it is a reference. If + // the function does not have a `self` parameter, this is `nullopt`. + std::optional self_param; }; } // namespace @@ -418,8 +425,8 @@ static auto BuildCppFunctionDeclForCarbonFn(Context& context, // Get parameters types. llvm::SmallVector cpp_param_types; - if (callee.has_self()) { - auto cpp_type = MapToCppType(context, callee.self_type_id); + if (callee.self_param) { + auto cpp_type = MapToCppType(context, callee.self_param->type_id); if (cpp_type.isNull()) { context.TODO(loc_id, "failed to map Carbon self type to C++"); return nullptr; @@ -497,6 +504,9 @@ static auto BuildCppToCarbonThunkDecl( clang::DeclarationNameInfo name_info(thunk_name, clang_loc); auto ext_proto_info = clang::FunctionProtoType::ExtProtoInfo(); + if (target.self_param && target.self_param->is_ref) { + ext_proto_info.RefQualifier = clang::RQ_LValue; + } clang::QualType thunk_function_type = ast_context.getFunctionType( cpp_return_type, thunk_param_types, ext_proto_info); @@ -587,7 +597,7 @@ static auto BuildCppToCarbonThunkBody(clang::Sema& sema, llvm::SmallVector call_args; // For methods, pass the `this` pointer as the first argument to the callee. - if (target.has_self()) { + if (target.self_param) { auto* parent_class = cast(target.decl_context); clang::QualType class_type = sema.getASTContext().getCanonicalTagType(parent_class); @@ -702,7 +712,7 @@ static auto BuildCarbonToCarbonThunk(Context& context, SemIR::LocId loc_id, context, loc_id, {.parent_scope_id = target.function.parent_scope_id, .name_id = thunk_name_id, - .self_type_id = target.self_type_id, + .self_type_id = target.GetSelfTypeId(), .param_type_ids = thunk_param_type_ids, .param_kind = ParamPatternKind::Ref}) .second; diff --git a/toolchain/check/testdata/interop/cpp/class/export/method.carbon b/toolchain/check/testdata/interop/cpp/class/export/method.carbon index 164f1ec2d38c..68cf7d32f324 100644 --- a/toolchain/check/testdata/interop/cpp/class/export/method.carbon +++ b/toolchain/check/testdata/interop/cpp/class/export/method.carbon @@ -68,7 +68,7 @@ void F() { } '''; -// --- todo_ref_method_rvalue.carbon +// --- fail_ref_method_rvalue.carbon library "[[@TEST_NAME]]"; import Cpp; @@ -78,7 +78,13 @@ class C { inline Cpp ''' void F() { - // TODO: this should be disallowed. + // CHECK:STDERR: fail_ref_method_rvalue.carbon:[[@LINE+7]]:3: error: 'this' argument to member function 'M' is an rvalue, but function has non-const lvalue ref-qualifier [CppInteropParseError] + // CHECK:STDERR: 17 | Carbon::C().M(); + // CHECK:STDERR: | ^ + // CHECK:STDERR: fail_ref_method_rvalue.carbon:[[@LINE-8]]:25: note: 'M' declared here [CppInteropParseNote] + // CHECK:STDERR: 5 | fn M[ref self: Self](); + // CHECK:STDERR: | ^ + // CHECK:STDERR: Carbon::C().M(); } ''';