Fix passing mode computation for methods. (#7225)

We were incorrectly computing the index of the Clang implicit conversion
corresponding to method arguments. This led to wrong code and a crash in
lowering due to a calling convention mismatch.

Fixes #7224.

Assisted-by: Gemini via Antigravity
This commit is contained in:
Richard Smith
2026-05-18 20:30:23 +00:00
committed by GitHub
parent 773ecdfac6
commit b87f848db4
3 changed files with 75 additions and 33 deletions
+5 -6
View File
@@ -243,12 +243,11 @@ auto ComputeClangDeclSignatureFromBestViableFunction(
// Compute which conversion sequence corresponds to this argument.
// TODO: Clang should expose a way to compute this.
int conversion_index = i;
if (auto* method = dyn_cast<clang::CXXMethodDecl>(candidate->Function)) {
if (method->isStatic()) {
// Static methods get an object parameter conversion at index 0, even
// though there's no argument.
++conversion_index;
}
if (isa<clang::CXXMethodDecl>(candidate->Function) &&
!isa<clang::CXXConstructorDecl>(candidate->Function)) {
// Methods (both static and non-static, but not constructors) get an
// object parameter conversion at index 0.
++conversion_index;
}
signature.passing_modes.push_back(GetPassingModeForCppParameter(
@@ -1189,7 +1189,6 @@ fn F() {
// CHECK:STDOUT: %int_3.1ba: Core.IntLiteral = int_value 3 [concrete]
// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [concrete]
// CHECK:STDOUT: %i32: type = class_type @Int, @Int(%int_32) [concrete]
// CHECK:STDOUT: %i32.builtin: type = int_type signed, %int_32 [concrete]
// CHECK:STDOUT: %pattern_type.7ce: type = pattern_type %i32 [concrete]
// CHECK:STDOUT: %operator_LessLess__carbon_thunk.type: type = fn_type @operator_LessLess__carbon_thunk [concrete]
// CHECK:STDOUT: %operator_LessLess__carbon_thunk: %operator_LessLess__carbon_thunk.type = struct_value () [concrete]
@@ -1253,12 +1252,8 @@ fn F() {
// CHECK:STDOUT: %Core.IntLiteral.as.ImplicitAs.impl.Convert.bound.577: <bound method> = bound_method %int_42.20e, %Core.IntLiteral.as.ImplicitAs.impl.Convert.f1a [concrete]
// CHECK:STDOUT: %bound_method.cd5: <bound method> = bound_method %int_42.20e, %Core.IntLiteral.as.ImplicitAs.impl.Convert.specific_fn [concrete]
// CHECK:STDOUT: %int_42.c68: %i32 = int_value 42 [concrete]
// CHECK:STDOUT: %.1a9: ref %i32 = temporary invalid, %int_42.c68 [concrete]
// CHECK:STDOUT: %Destroy.Op.type.bae255.2: type = fn_type @Destroy.Op.loc48_30.2 [concrete]
// CHECK:STDOUT: %Destroy.Op.651ba6.2: %Destroy.Op.type.bae255.2 = struct_value () [concrete]
// CHECK:STDOUT: %Destroy.Op.bound: <bound method> = bound_method %.1a9, %Destroy.Op.651ba6.2 [concrete]
// CHECK:STDOUT: %Destroy.Op.type.bae255.3: type = fn_type @Destroy.Op.loc45 [concrete]
// CHECK:STDOUT: %Destroy.Op.651ba6.3: %Destroy.Op.type.bae255.3 = struct_value () [concrete]
// CHECK:STDOUT: %Destroy.Op.type: type = fn_type @Destroy.Op [concrete]
// CHECK:STDOUT: %Destroy.Op: %Destroy.Op.type = struct_value () [concrete]
// CHECK:STDOUT: %C.cpp_destructor.type: type = fn_type @C.cpp_destructor [concrete]
// CHECK:STDOUT: %C.cpp_destructor: %C.cpp_destructor.type = struct_value () [concrete]
// CHECK:STDOUT: %C.Op.type: type = fn_type @C.Op [concrete]
@@ -1880,26 +1875,24 @@ fn F() {
// CHECK:STDOUT: %specific_fn.loc48: <specific function> = specific_function %impl.elem0.loc48, @Core.IntLiteral.as.ImplicitAs.impl.Convert(constants.%int_32) [concrete = constants.%Core.IntLiteral.as.ImplicitAs.impl.Convert.specific_fn]
// CHECK:STDOUT: %bound_method.loc48_30.2: <bound method> = bound_method %int_42, %specific_fn.loc48 [concrete = constants.%bound_method.cd5]
// CHECK:STDOUT: %Core.IntLiteral.as.ImplicitAs.impl.Convert.call.loc48: init %i32 = call %bound_method.loc48_30.2(%int_42) [concrete = constants.%int_42.c68]
// CHECK:STDOUT: %.loc48_30.1: init %i32 = converted %int_42, %Core.IntLiteral.as.ImplicitAs.impl.Convert.call.loc48 [concrete = constants.%int_42.c68]
// CHECK:STDOUT: %.loc48_30.2: ref %i32 = temporary_storage
// CHECK:STDOUT: %.loc48_30.3: ref %i32 = temporary %.loc48_30.2, %.loc48_30.1 [concrete = constants.%.1a9]
// CHECK:STDOUT: %C.cpp_operator.call: init %i32 = call %C.cpp_operator.bound(%c1.ref.loc48, %.loc48_30.3)
// CHECK:STDOUT: %.loc48_30.1: %i32 = value_of_initializer %Core.IntLiteral.as.ImplicitAs.impl.Convert.call.loc48 [concrete = constants.%int_42.c68]
// CHECK:STDOUT: %.loc48_30.2: %i32 = converted %int_42, %.loc48_30.1 [concrete = constants.%int_42.c68]
// CHECK:STDOUT: %C.cpp_operator.call: init %i32 = call %C.cpp_operator.bound(%c1.ref.loc48, %.loc48_30.2)
// CHECK:STDOUT: %i32: type = type_literal constants.%i32 [concrete = constants.%i32]
// CHECK:STDOUT: %.loc48_32.1: %i32 = value_of_initializer %C.cpp_operator.call
// CHECK:STDOUT: %.loc48_32.2: %i32 = converted %C.cpp_operator.call, %.loc48_32.1
// CHECK:STDOUT: %index: %i32 = value_binding index, %.loc48_32.2
// CHECK:STDOUT: %Destroy.Op.call.loc48: init %empty_tuple.type = call constants.%Destroy.Op.bound(constants.%.1a9)
// CHECK:STDOUT: %Destroy.Op.bound.loc45: <bound method> = bound_method %.loc45_44.3, constants.%Destroy.Op.651ba6.3
// CHECK:STDOUT: %Destroy.Op.bound.loc45: <bound method> = bound_method %.loc45_44.3, constants.%Destroy.Op
// CHECK:STDOUT: %Destroy.Op.call.loc45: init %empty_tuple.type = call %Destroy.Op.bound.loc45(%.loc45_44.3)
// CHECK:STDOUT: %Destroy.Op.bound.loc44: <bound method> = bound_method %.loc44_47.3, constants.%Destroy.Op.651ba6.3
// CHECK:STDOUT: %Destroy.Op.bound.loc44: <bound method> = bound_method %.loc44_47.3, constants.%Destroy.Op
// CHECK:STDOUT: %Destroy.Op.call.loc44: init %empty_tuple.type = call %Destroy.Op.bound.loc44(%.loc44_47.3)
// CHECK:STDOUT: %Destroy.Op.bound.loc43: <bound method> = bound_method %.loc43_35.3, constants.%Destroy.Op.651ba6.3
// CHECK:STDOUT: %Destroy.Op.bound.loc43: <bound method> = bound_method %.loc43_35.3, constants.%Destroy.Op
// CHECK:STDOUT: %Destroy.Op.call.loc43: init %empty_tuple.type = call %Destroy.Op.bound.loc43(%.loc43_35.3)
// CHECK:STDOUT: %Destroy.Op.bound.loc42: <bound method> = bound_method %.loc42_38.3, constants.%Destroy.Op.651ba6.3
// CHECK:STDOUT: %Destroy.Op.bound.loc42: <bound method> = bound_method %.loc42_38.3, constants.%Destroy.Op
// CHECK:STDOUT: %Destroy.Op.call.loc42: init %empty_tuple.type = call %Destroy.Op.bound.loc42(%.loc42_38.3)
// CHECK:STDOUT: %Destroy.Op.bound.loc41: <bound method> = bound_method %.loc41_35.3, constants.%Destroy.Op.651ba6.3
// CHECK:STDOUT: %Destroy.Op.bound.loc41: <bound method> = bound_method %.loc41_35.3, constants.%Destroy.Op
// CHECK:STDOUT: %Destroy.Op.call.loc41: init %empty_tuple.type = call %Destroy.Op.bound.loc41(%.loc41_35.3)
// CHECK:STDOUT: %Destroy.Op.bound.loc40: <bound method> = bound_method %.loc40_31.3, constants.%Destroy.Op.651ba6.3
// CHECK:STDOUT: %Destroy.Op.bound.loc40: <bound method> = bound_method %.loc40_31.3, constants.%Destroy.Op
// CHECK:STDOUT: %Destroy.Op.call.loc40: init %empty_tuple.type = call %Destroy.Op.bound.loc40(%.loc40_31.3)
// CHECK:STDOUT: <elided>
// CHECK:STDOUT: %C.Op.bound.loc23: <bound method> = bound_method %.loc23_38.3, constants.%C.Op
@@ -1953,14 +1946,7 @@ fn F() {
// CHECK:STDOUT: <elided>
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Destroy.Op.loc48_30.1(%self.param: ref %i32.builtin) = "no_op";
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Destroy.Op.loc48_30.2(%self.param: ref %i32) {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Destroy.Op.loc45(%self.param: ref bool) = "no_op";
// CHECK:STDOUT: fn @Destroy.Op(%self.param: ref bool) = "no_op";
// CHECK:STDOUT:
// CHECK:STDOUT: --- multiple_calls.carbon
// CHECK:STDOUT:
@@ -3189,7 +3175,9 @@ fn F() {
// CHECK:STDOUT: %c2.ref: ref %C = name_ref c2, %c2
// CHECK:STDOUT: %C.cpp_operator.bound.loc10: <bound method> = bound_method %c1.ref.loc10, imports.%C.cpp_operator.decl.828f43.2
// CHECK:STDOUT: %.loc10_3: ref %C = splice_block %c3.var {}
// CHECK:STDOUT: %addr.loc10_29.1: %ptr.d9e = addr_of %c2.ref
// CHECK:STDOUT: %.loc10_31.1: %C = acquire_value %c2.ref
// CHECK:STDOUT: %.loc10_31.2: ref %C = value_as_ref %.loc10_31.1
// CHECK:STDOUT: %addr.loc10_29.1: %ptr.d9e = addr_of %.loc10_31.2
// CHECK:STDOUT: %addr.loc10_29.2: %ptr.d9e = addr_of %.loc10_3
// CHECK:STDOUT: %operator_Plus__carbon_thunk.call: init %empty_tuple.type = call imports.%operator_Plus__carbon_thunk.decl(%c1.ref.loc10, %addr.loc10_29.1, %addr.loc10_29.2)
// CHECK:STDOUT: %.loc10_29: init %C to %.loc10_3 = mark_in_place_init %operator_Plus__carbon_thunk.call
+55
View File
@@ -73,6 +73,21 @@ fn Call(n: Cpp.NeedThunk) {
n.Explicit(1);
}
// --- call_with_ref_self.carbon
library "[[@TEST_NAME]]";
import Cpp;
inline Cpp '''
struct A {
void f(int x, int y);
};
''';
fn F(ref r: Cpp.A) {
r.f(1, 2);
}
// CHECK:STDOUT: ; ModuleID = 'call_by_val.carbon'
// CHECK:STDOUT: source_filename = "call_by_val.carbon"
// CHECK:STDOUT: target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
@@ -383,3 +398,43 @@ fn Call(n: Cpp.NeedThunk) {
// CHECK:STDOUT: !33 = !{!34}
// CHECK:STDOUT: !34 = !DILocalVariable(arg: 1, scope: !29, type: !32)
// CHECK:STDOUT: !35 = !DILocation(line: 8, column: 14, scope: !29)
// CHECK:STDOUT: ; ModuleID = 'call_with_ref_self.carbon'
// CHECK:STDOUT: source_filename = "call_with_ref_self.carbon"
// CHECK:STDOUT: target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
// CHECK:STDOUT: target triple = "x86_64-unknown-linux-gnu"
// CHECK:STDOUT:
// CHECK:STDOUT: ; Function Attrs: nounwind
// CHECK:STDOUT: define void @_CF.Main(ptr %r) #0 !dbg !11 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: call void @_ZN1A1fEii(ptr %r, i32 1, i32 2), !dbg !17
// CHECK:STDOUT: ret void, !dbg !18
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: declare void @_ZN1A1fEii(ptr noundef nonnull align 1 dereferenceable(1), i32 noundef, i32 noundef) #1
// CHECK:STDOUT:
// CHECK:STDOUT: attributes #0 = { nounwind }
// CHECK:STDOUT: attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
// CHECK:STDOUT:
// CHECK:STDOUT: !llvm.module.flags = !{!0, !1, !2, !3, !4}
// CHECK:STDOUT: !llvm.dbg.cu = !{!5}
// CHECK:STDOUT: !llvm.errno.tbaa = !{!7}
// CHECK:STDOUT:
// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
// CHECK:STDOUT: !2 = !{i32 8, !"PIC Level", i32 2}
// CHECK:STDOUT: !3 = !{i32 7, !"PIE Level", i32 2}
// CHECK:STDOUT: !4 = !{i32 7, !"uwtable", i32 2}
// CHECK:STDOUT: !5 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !6, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
// CHECK:STDOUT: !6 = !DIFile(filename: "call_with_ref_self.carbon", directory: "")
// CHECK:STDOUT: !7 = !{!8, !8, i64 0}
// CHECK:STDOUT: !8 = !{!"int", !9, i64 0}
// CHECK:STDOUT: !9 = !{!"omnipotent char", !10, i64 0}
// CHECK:STDOUT: !10 = !{!"Simple C++ TBAA"}
// CHECK:STDOUT: !11 = distinct !DISubprogram(name: "F", linkageName: "_CF.Main", scope: null, file: !6, line: 11, type: !12, spFlags: DISPFlagDefinition, unit: !5, retainedNodes: !15)
// CHECK:STDOUT: !12 = !DISubroutineType(types: !13)
// CHECK:STDOUT: !13 = !{null, !14}
// CHECK:STDOUT: !14 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
// CHECK:STDOUT: !15 = !{!16}
// CHECK:STDOUT: !16 = !DILocalVariable(arg: 1, scope: !11, type: !14)
// CHECK:STDOUT: !17 = !DILocation(line: 12, column: 3, scope: !11)
// CHECK:STDOUT: !18 = !DILocation(line: 11, column: 1, scope: !11)