Prevent `ref self` methods from being called from C++ with an rvalue (#7130)

If the Carbon method takes a `ref self`, give the C++ thunk an lvalue
ref-qualifier.

toolchain/check/cpp/export.cpp

@@ -4,6 +4,8 @@
 
 #include "toolchain/check/cpp/export.h"
 
+#include <optional>
+
 #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<SemIR::RefParamPattern>(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<SemIR::RefParamPattern>(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<Param> 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<Param> self_param;
 };
 }  // namespace
 
@@ -418,8 +425,8 @@ static auto BuildCppFunctionDeclForCarbonFn(Context& context,
 
   // Get parameters types.
   llvm::SmallVector<clang::QualType> 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<clang::Expr*> 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<clang::CXXRecordDecl>(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;

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();
 }
 ''';