Skip to content

Commit e2985b6

Browse files
egorzhdanegorzhdan
authored
Merge pull request #38618 from egorzhdan/cxx-mutating
C++ Interop: import const methods as non-mutating
2 parents 77cb4b6 + a8f126f commit e2985b6

23 files changed

+200
-128
lines changed

‎include/swift/AST/ClangModuleLoader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ class ClangModuleLoader : public ModuleLoader {
236236
instantiateCXXFunctionTemplate(ASTContext &ctx,
237237
clang::FunctionTemplateDecl *func,
238238
SubstitutionMap subst) = 0;
239+
240+
virtual bool isCXXMethodMutating(const clang::CXXMethodDecl *method) = 0;
239241
};
240242

241243
/// Describes a C++ template instantiation error.

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,8 @@ class ClangImporter final : public ClangModuleLoader {
486486
instantiateCXXFunctionTemplate(ASTContext &ctx,
487487
clang::FunctionTemplateDecl *func,
488488
SubstitutionMap subst) override;
489+
490+
bool isCXXMethodMutating(const clang::CXXMethodDecl *method) override;
489491
};
490492

491493
ImportDecl *createImportDecl(ASTContext &Ctx, DeclContext *DC, ClangNode ClangN,

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4322,3 +4322,8 @@ ClangImporter::instantiateCXXClassTemplate(
43224322
return dyn_cast_or_null<StructDecl>(
43234323
Impl.importDecl(ctsd, Impl.CurrentVersion));
43244324
}
4325+
4326+
bool ClangImporter::isCXXMethodMutating(const clang::CXXMethodDecl *method) {
4327+
return isa<clang::CXXConstructorDecl>(method) || !method->isConst() ||
4328+
method->getParent()->hasMutableFields();
4329+
}

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4176,10 +4176,10 @@ namespace {
41764176
selfIdx = None;
41774177
} else {
41784178
selfIdx = 0;
4179-
// Workaround until proper const support is handled: Force
4180-
// everything to be mutating. This implicitly makes the parameter
4181-
// indirect.
4182-
selfIsInOut = true;
4179+
// If the method is imported as mutating, this implicitly makes the
4180+
// parameter indirect.
4181+
selfIsInOut = Impl.SwiftContext.getClangModuleLoader()
4182+
->isCXXMethodMutating(mdecl);
41834183
}
41844184
}
41854185
}
@@ -7588,23 +7588,24 @@ SwiftDeclConverter::importAccessor(const clang::ObjCMethodDecl *clangAccessor,
75887588
return accessor;
75897589
}
75907590

7591-
static InOutExpr *
7592-
createInOutSelfExpr(AccessorDecl *accessorDecl) {
7591+
static Expr *createSelfExpr(AccessorDecl *accessorDecl) {
75937592
ASTContext &ctx = accessorDecl->getASTContext();
75947593

7595-
auto inoutSelfDecl = accessorDecl->getImplicitSelfDecl();
7596-
auto inoutSelfRefExpr =
7597-
new (ctx) DeclRefExpr(inoutSelfDecl, DeclNameLoc(),
7598-
/*implicit=*/ true);
7599-
inoutSelfRefExpr->setType(LValueType::get(inoutSelfDecl->getInterfaceType()));
7600-
7601-
auto inoutSelfExpr =
7602-
new (ctx) InOutExpr(SourceLoc(),
7603-
inoutSelfRefExpr,
7604-
accessorDecl->mapTypeIntoContext(
7605-
inoutSelfDecl->getValueInterfaceType()),
7606-
/*isImplicit=*/ true);
7607-
inoutSelfExpr->setType(InOutType::get(inoutSelfDecl->getInterfaceType()));
7594+
auto selfDecl = accessorDecl->getImplicitSelfDecl();
7595+
auto selfRefExpr = new (ctx) DeclRefExpr(selfDecl, DeclNameLoc(),
7596+
/*implicit*/ true);
7597+
7598+
if (!accessorDecl->isMutating()) {
7599+
selfRefExpr->setType(selfDecl->getInterfaceType());
7600+
return selfRefExpr;
7601+
}
7602+
selfRefExpr->setType(LValueType::get(selfDecl->getInterfaceType()));
7603+
7604+
auto inoutSelfExpr = new (ctx) InOutExpr(
7605+
SourceLoc(), selfRefExpr,
7606+
accessorDecl->mapTypeIntoContext(selfDecl->getValueInterfaceType()),
7607+
/*isImplicit*/ true);
7608+
inoutSelfExpr->setType(InOutType::get(selfDecl->getInterfaceType()));
76087609
return inoutSelfExpr;
76097610
}
76107611

@@ -7622,7 +7623,7 @@ createParamRefExpr(AccessorDecl *accessorDecl, unsigned index) {
76227623

76237624
static CallExpr *
76247625
createAccessorImplCallExpr(FuncDecl *accessorImpl,
7625-
InOutExpr *inoutSelfExpr,
7626+
Expr *selfExpr,
76267627
DeclRefExpr *keyRefExpr) {
76277628
ASTContext &ctx = accessorImpl->getASTContext();
76287629

@@ -7633,9 +7634,7 @@ createAccessorImplCallExpr(FuncDecl *accessorImpl,
76337634
accessorImplExpr->setType(accessorImpl->getInterfaceType());
76347635

76357636
auto accessorImplDotCallExpr =
7636-
new (ctx) DotSyntaxCallExpr(accessorImplExpr,
7637-
SourceLoc(),
7638-
inoutSelfExpr);
7637+
new (ctx) DotSyntaxCallExpr(accessorImplExpr, SourceLoc(), selfExpr);
76397638
accessorImplDotCallExpr->setType(accessorImpl->getMethodInterfaceType());
76407639
accessorImplDotCallExpr->setThrows(false);
76417640

@@ -7655,14 +7654,13 @@ synthesizeSubscriptGetterBody(AbstractFunctionDecl *afd, void *context) {
76557654

76567655
ASTContext &ctx = getterDecl->getASTContext();
76577656

7658-
InOutExpr *inoutSelfExpr = createInOutSelfExpr(getterDecl);
7657+
Expr *selfExpr = createSelfExpr(getterDecl);
76597658
DeclRefExpr *keyRefExpr = createParamRefExpr(getterDecl, 0);
76607659

76617660
Type elementTy = getterDecl->getResultInterfaceType();
76627661

7663-
auto *getterImplCallExpr = createAccessorImplCallExpr(getterImpl,
7664-
inoutSelfExpr,
7665-
keyRefExpr);
7662+
auto *getterImplCallExpr =
7663+
createAccessorImplCallExpr(getterImpl, selfExpr, keyRefExpr);
76667664

76677665
// This default handles C++'s operator[] that returns a value type.
76687666
Expr *propertyExpr = getterImplCallExpr;
@@ -7708,14 +7706,13 @@ synthesizeSubscriptSetterBody(AbstractFunctionDecl *afd, void *context) {
77087706

77097707
ASTContext &ctx = setterDecl->getASTContext();
77107708

7711-
InOutExpr *inoutSelfExpr = createInOutSelfExpr(setterDecl);
7709+
Expr *selfExpr = createSelfExpr(setterDecl);
77127710
DeclRefExpr *valueParamRefExpr = createParamRefExpr(setterDecl, 0);
77137711
DeclRefExpr *keyParamRefExpr = createParamRefExpr(setterDecl, 1);
77147712

77157713
Type elementTy = valueParamRefExpr->getDecl()->getInterfaceType();
77167714

7717-
auto *setterImplCallExpr = createAccessorImplCallExpr(setterImpl,
7718-
inoutSelfExpr,
7715+
auto *setterImplCallExpr = createAccessorImplCallExpr(setterImpl, selfExpr,
77197716
keyParamRefExpr);
77207717

77217718
VarDecl *pointeePropertyDecl = ctx.getPointerPointeePropertyDecl(PTK_UnsafeMutablePointer);

‎lib/ClangImporter/ImportType.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1880,9 +1880,12 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
18801880

18811881
param->setInterfaceType(parentType.getType());
18821882

1883-
// Workaround until proper const support is handled: Force everything to
1884-
// be mutating. This implicitly makes the parameter indirect.
1885-
param->setSpecifier(ParamSpecifier::InOut);
1883+
if (SwiftContext.getClangModuleLoader()->isCXXMethodMutating(CMD)) {
1884+
// This implicitly makes the parameter indirect.
1885+
param->setSpecifier(ParamSpecifier::InOut);
1886+
} else {
1887+
param->setSpecifier(ParamSpecifier::Default);
1888+
}
18861889

18871890
parameters.push_back(param);
18881891
}

‎lib/SIL/IR/SILFunctionType.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2830,18 +2830,31 @@ class CFunctionConventions : public CFunctionTypeConventions {
28302830
class CXXMethodConventions : public CFunctionTypeConventions {
28312831
using super = CFunctionTypeConventions;
28322832
const clang::CXXMethodDecl *TheDecl;
2833+
bool isMutating;
28332834

28342835
public:
2835-
CXXMethodConventions(const clang::CXXMethodDecl *decl)
2836+
CXXMethodConventions(const clang::CXXMethodDecl *decl, bool isMutating)
28362837
: CFunctionTypeConventions(
28372838
ConventionsKind::CXXMethod,
28382839
decl->getType()->castAs<clang::FunctionType>()),
2839-
TheDecl(decl) {}
2840+
TheDecl(decl), isMutating(isMutating) {}
28402841
ParameterConvention
28412842
getIndirectSelfParameter(const AbstractionPattern &type) const override {
2842-
if (TheDecl->isConst())
2843+
llvm_unreachable(
2844+
"cxx functions do not have a Swift self parameter; "
2845+
"foreign self parameter is handled in getIndirectParameter");
2846+
}
2847+
2848+
ParameterConvention
2849+
getIndirectParameter(unsigned int index, const AbstractionPattern &type,
2850+
const TypeLowering &substTL) const override {
2851+
// `self` is the last parameter.
2852+
if (index == TheDecl->getNumParams()) {
2853+
if (isMutating)
2854+
return ParameterConvention::Indirect_Inout;
28432855
return ParameterConvention::Indirect_In_Guaranteed;
2844-
return ParameterConvention::Indirect_Inout;
2856+
}
2857+
return super::getIndirectParameter(index, type, substTL);
28452858
}
28462859
ResultConvention getResult(const TypeLowering &resultTL) const override {
28472860
if (isa<clang::CXXConstructorDecl>(TheDecl)) {
@@ -2893,7 +2906,9 @@ static CanSILFunctionType getSILFunctionTypeForClangDecl(
28932906
AbstractionPattern origPattern = method->isOverloadedOperator() ?
28942907
AbstractionPattern::getCXXOperatorMethod(origType, method, foreignInfo.Self):
28952908
AbstractionPattern::getCXXMethod(origType, method, foreignInfo.Self);
2896-
auto conventions = CXXMethodConventions(method);
2909+
bool isMutating =
2910+
TC.Context.getClangModuleLoader()->isCXXMethodMutating(method);
2911+
auto conventions = CXXMethodConventions(method, isMutating);
28972912
return getSILFunctionType(TC, TypeExpansionContext::minimal(), origPattern,
28982913
substInterfaceType, extInfoBuilder, conventions,
28992914
foreignInfo, constant, constant, None,

‎test/ClangImporter/cxx_interop_ir.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
// RUN: %target-swiftxx-frontend -module-name cxx_ir -I %S/Inputs/custom-modules -emit-ir -o - -primary-file %s | %FileCheck %s
2+
//
3+
// We can't yet call member functions correctly on Windows (SR-13129).
4+
// XFAIL: OS=windows-msvc
25

36
import CXXInterop
47

@@ -40,9 +43,10 @@ func basicMethods(a: UnsafeMutablePointer<Methods>) -> Int32 {
4043
}
4144

4245
// CHECK-LABEL: define hidden swiftcc i32 @"$s6cxx_ir17basicMethodsConst1as5Int32VSpySo0D0VG_tF"(i8* %0)
43-
// CHECK: [[THIS_PTR1:%.*]] = bitcast i8* %0 to %TSo7MethodsV*
46+
// CHECK: [[THIS_PTR1:%.*]] = alloca %TSo7MethodsV, align 8
4447
// CHECK: [[THIS_PTR2:%.*]] = bitcast %TSo7MethodsV* [[THIS_PTR1]] to %class.Methods*
45-
// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZNK7Methods17SimpleConstMethodEi|"\?SimpleConstMethod@Methods@@QEBAHH@Z"}}(%class.Methods* [[THIS_PTR2]], i32{{( signext)?}} 3)
48+
// CHECK: [[THIS_PTR3:%.*]] = bitcast %TSo7MethodsV* [[THIS_PTR1]] to %class.Methods*
49+
// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZNK7Methods17SimpleConstMethodEi|"\?SimpleConstMethod@Methods@@QEBAHH@Z"}}(%class.Methods* [[THIS_PTR3]], i32{{( signext)?}} 3)
4650
// CHECK: ret i32 [[RESULT]]
4751
func basicMethodsConst(a: UnsafeMutablePointer<Methods>) -> Int32 {
4852
return a.pointee.SimpleConstMethod(3)

‎test/Interop/Cxx/class/Inputs/module.modulemap

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ module MemberVariables {
4848
requires cplusplus
4949
}
5050

51+
module MutableMembers {
52+
header "mutable-members.h"
53+
requires cplusplus
54+
}
55+
5156
module ProtocolConformance {
5257
header "protocol-conformance.h"
5358
requires cplusplus

‎test/Interop/Cxx/class/Inputs/mutable-members.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_MUTABLE_MEMBERS_H
2+
#define TEST_INTEROP_CXX_CLASS_INPUTS_MUTABLE_MEMBERS_H
3+
4+
struct HasPublicMutableMember {
5+
mutable int a = 0;
6+
7+
int foo() const {
8+
a++;
9+
return a;
10+
}
11+
};
12+
13+
struct HasPrivateMutableMember {
14+
private:
15+
mutable int a = 0;
16+
17+
public:
18+
void bar() const { a++; }
19+
};
20+
21+
#endif // TEST_INTEROP_CXX_CLASS_INPUTS_MUTABLE_MEMBERS_H

‎test/Interop/Cxx/class/mutable-members-module-interface.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// RUN: %target-swift-ide-test -print-module -module-to-print=MutableMembers -I %S/Inputs -source-filename=x -enable-cxx-interop | %FileCheck %s
2+
3+
// CHECK: struct HasPublicMutableMember {
4+
// CHECK: var a: Int32
5+
// CHECK: mutating func foo() -> Int32
6+
// CHECK: }
7+
8+
// CHECK: struct HasPrivateMutableMember {
9+
// CHECK: mutating func bar()
10+
// CHECK: }

‎test/Interop/Cxx/class/mutable-members-typechecker.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-cxx-interop
2+
3+
import MutableMembers
4+
5+
let obj = HasPublicMutableMember(a: 42) // expected-note {{change 'let' to 'var' to make it mutable}}
6+
let i = obj.foo() // expected-error {{cannot use mutating member on immutable value: 'obj' is a 'let' constant}}

‎test/Interop/Cxx/class/mutable-members.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-cxx-interop)
2+
//
3+
// REQUIRES: executable_test
4+
5+
import StdlibUnittest
6+
import MutableMembers
7+
8+
var MembersTestSuite = TestSuite("MembersTestSuite")
9+
10+
MembersTestSuite.test("MutableMembers") {
11+
var obj = HasPublicMutableMember(a: 1)
12+
expectEqual(obj.foo(), 2)
13+
expectEqual(obj.foo(), 3)
14+
}
15+
16+
runAllTests()

0 commit comments

Comments
 (0)