Skip to content

Commit 2aac886

Browse files
author
Harlan
authored
[InterfaceGen] Only print 'mutating' and 'nonmutating' on accessors (#19459)
* [InterfaceGen] Only print 'mutating' and 'nonmutating' on accessors * Add SILGen test for usage of dynamic accessors in and out of interfaces * Add -enable-objc-interop to dynamic_accessors test
1 parent b505b62 commit 2aac886

File tree

6 files changed

+148
-62
lines changed

6 files changed

+148
-62
lines changed

‎include/swift/AST/Decl.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5471,10 +5471,6 @@ class FuncDecl : public AbstractFunctionDecl {
54715471
Bits.FuncDecl.SelfAccess = static_cast<unsigned>(mod);
54725472
}
54735473

5474-
/// \returns true if this is non-mutating due to applying a 'mutating'
5475-
/// attribute. For example a "mutating set" accessor.
5476-
bool isExplicitNonMutating() const;
5477-
54785474
SourceLoc getStaticLoc() const { return StaticLoc; }
54795475
SourceLoc getFuncLoc() const { return FuncLoc; }
54805476

@@ -5682,6 +5678,13 @@ class AccessorDecl final : public FuncDecl {
56825678
llvm_unreachable("bad accessor kind");
56835679
}
56845680

5681+
/// \returns true if this is non-mutating due to applying a 'mutating'
5682+
/// attribute. For example a "mutating set" accessor.
5683+
bool isExplicitNonMutating() const;
5684+
5685+
/// Is the accesor one of the kinds that's assumed nonmutating by default?
5686+
bool isAssumedNonMutating() const;
5687+
56855688
/// Is this accessor one of the kinds that's implicitly a coroutine?
56865689
bool isCoroutine() const {
56875690
switch (getAccessorKind()) {

‎lib/AST/ASTPrinter.cpp

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
659659
void printAttributes(const Decl *D);
660660
void printTypedPattern(const TypedPattern *TP);
661661
void printBraceStmt(const BraceStmt *stmt, bool newlineIfEmpty = true);
662+
void printAccessorDecl(const AccessorDecl *decl);
662663

663664
public:
664665
void printPattern(const Pattern *pattern);
@@ -692,7 +693,8 @@ class PrintAST : public ASTVisitor<PrintAST> {
692693
bool shouldPrint(const Decl *D, bool Notify = false);
693694
bool shouldPrintPattern(const Pattern *P);
694695
void printPatternType(const Pattern *P);
695-
void printAccessors(AbstractStorageDecl *ASD);
696+
void printAccessors(const AbstractStorageDecl *ASD);
697+
void printMutatingModifiersIfNeeded(const AccessorDecl *accessor);
696698
void printMembersOfDecl(Decl * NTD, bool needComma = false,
697699
bool openBracket = true, bool closeBracket = true);
698700
void printMembers(ArrayRef<Decl *> members, bool needComma = false,
@@ -813,6 +815,16 @@ static StaticSpellingKind getCorrectStaticSpelling(const Decl *D) {
813815
}
814816
}
815817

818+
static bool hasMutatingGetter(const AbstractStorageDecl *ASD) {
819+
return ASD->getGetter() && ASD->isGetterMutating();
820+
}
821+
822+
static bool hasNonMutatingSetter(const AbstractStorageDecl *ASD) {
823+
if (!ASD->isSettable(nullptr)) return false;
824+
auto setter = ASD->getSetter();
825+
return setter && setter->isExplicitNonMutating();
826+
}
827+
816828
void PrintAST::printAttributes(const Decl *D) {
817829
if (Options.SkipAttributes)
818830
return;
@@ -825,9 +837,14 @@ void PrintAST::printAttributes(const Decl *D) {
825837
Options.ExcludeAttrList.push_back(DAK_Final);
826838
}
827839

828-
// Don't print 'final' on accessors.
840+
// Don't print any contextual decl modifiers.
841+
// We will handle 'mutating' and 'nonmutating' separately.
829842
if (Options.PrintImplicitAttrs && isa<AccessorDecl>(D)) {
830-
Options.ExcludeAttrList.push_back(DAK_Final);
843+
#define EXCLUDE_ATTR(Class) Options.ExcludeAttrList.push_back(DAK_##Class);
844+
#define CONTEXTUAL_DECL_ATTR(X, Class, Y, Z) EXCLUDE_ATTR(Class)
845+
#define CONTEXTUAL_SIMPLE_DECL_ATTR(X, Class, Y, Z) EXCLUDE_ATTR(Class)
846+
#define CONTEXTUAL_DECL_ATTR_ALIAS(X, Class) EXCLUDE_ATTR(Class)
847+
#include "swift/AST/Attr.def"
831848
}
832849

833850
// If the declaration is implicitly @objc, print the attribute now.
@@ -842,6 +859,12 @@ void PrintAST::printAttributes(const Decl *D) {
842859

843860
D->getAttrs().print(Printer, Options, D);
844861

862+
// Explicitly print 'mutating' and 'nonmutating' before getters and setters
863+
// for which that is true.
864+
if (auto accessor = dyn_cast<AccessorDecl>(D)) {
865+
printMutatingModifiersIfNeeded(accessor);
866+
}
867+
845868
Options.ExcludeAttrList.resize(originalExcludeAttrCount);
846869
}
847870

@@ -1514,23 +1537,6 @@ void PrintAST::printBodyIfNecessary(const AbstractFunctionDecl *decl) {
15141537
printBraceStmt(decl->getBody(), /*newlineIfEmpty*/!isa<AccessorDecl>(decl));
15151538
}
15161539

1517-
static bool isAccessorAssumedNonMutating(AccessorKind kind) {
1518-
switch (kind) {
1519-
case AccessorKind::Get:
1520-
case AccessorKind::Address:
1521-
case AccessorKind::Read:
1522-
return true;
1523-
1524-
case AccessorKind::Set:
1525-
case AccessorKind::WillSet:
1526-
case AccessorKind::DidSet:
1527-
case AccessorKind::MutableAddress:
1528-
case AccessorKind::Modify:
1529-
return false;
1530-
}
1531-
llvm_unreachable("bad addressor kind");
1532-
}
1533-
15341540
static StringRef getAccessorLabel(AccessorDecl *accessor) {
15351541
switch (accessor->getAccessorKind()) {
15361542
#define SINGLETON_ACCESSOR(ID, KEYWORD) \
@@ -1561,7 +1567,17 @@ static StringRef getAccessorLabel(AccessorDecl *accessor) {
15611567
llvm_unreachable("bad accessor kind");
15621568
}
15631569

1564-
void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
1570+
void PrintAST::printMutatingModifiersIfNeeded(const AccessorDecl *accessor) {
1571+
if (accessor->isAssumedNonMutating() && accessor->isMutating()) {
1572+
Printer.printKeyword("mutating");
1573+
Printer << " ";
1574+
} else if (accessor->isExplicitNonMutating()) {
1575+
Printer.printKeyword("nonmutating");
1576+
Printer << " ";
1577+
}
1578+
}
1579+
1580+
void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
15651581
if (isa<VarDecl>(ASD) && !Options.PrintPropertyAccessors)
15661582
return;
15671583

@@ -1590,13 +1606,9 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
15901606
bool inProtocol = isa<ProtocolDecl>(ASD->getDeclContext());
15911607
if ((inProtocol && !Options.PrintAccessorBodiesInProtocols) ||
15921608
PrintAbstract) {
1593-
bool mutatingGetter = ASD->getGetter() && ASD->isGetterMutating();
15941609
bool settable = ASD->isSettable(nullptr);
1595-
bool nonmutatingSetter = false;
1596-
if (settable && !ASD->isSetterMutating() && ASD->isInstanceMember() &&
1597-
!ASD->getDeclContext()->getDeclaredInterfaceType()
1598-
->hasReferenceSemantics())
1599-
nonmutatingSetter = true;
1610+
bool mutatingGetter = hasMutatingGetter(ASD);
1611+
bool nonmutatingSetter = hasNonMutatingSetter(ASD);
16001612

16011613
// We're about to print something like this:
16021614
// { mutating? get (nonmutating? set)? }
@@ -1660,18 +1672,8 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
16601672
if (!Accessor || !shouldPrint(Accessor))
16611673
return true;
16621674
if (!PrintAccessorBody) {
1663-
if (isAccessorAssumedNonMutating(Accessor->getAccessorKind())) {
1664-
if (Accessor->isMutating()) {
1665-
Printer << " ";
1666-
Printer.printKeyword("mutating");
1667-
}
1668-
} else {
1669-
if (Accessor->isExplicitNonMutating()) {
1670-
Printer << " ";
1671-
Printer.printKeyword("nonmutating");
1672-
}
1673-
}
16741675
Printer << " ";
1676+
printMutatingModifiersIfNeeded(Accessor);
16751677
Printer.printKeyword(getAccessorLabel(Accessor));
16761678
} else {
16771679
{

‎lib/AST/Decl.cpp

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5562,6 +5562,30 @@ AccessorDecl *AccessorDecl::create(ASTContext &ctx,
55625562
return D;
55635563
}
55645564

5565+
bool AccessorDecl::isAssumedNonMutating() const {
5566+
switch (getAccessorKind()) {
5567+
case AccessorKind::Get:
5568+
case AccessorKind::Address:
5569+
case AccessorKind::Read:
5570+
return true;
5571+
5572+
case AccessorKind::Set:
5573+
case AccessorKind::WillSet:
5574+
case AccessorKind::DidSet:
5575+
case AccessorKind::MutableAddress:
5576+
case AccessorKind::Modify:
5577+
return false;
5578+
}
5579+
llvm_unreachable("bad accessor kind");
5580+
}
5581+
5582+
bool AccessorDecl::isExplicitNonMutating() const {
5583+
return !isMutating() &&
5584+
!isAssumedNonMutating() &&
5585+
isInstanceMember() &&
5586+
!getDeclContext()->getDeclaredInterfaceType()->hasReferenceSemantics();
5587+
}
5588+
55655589
StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
55665590
assert(getDeclContext()->isTypeContext());
55675591
if (!isStatic())
@@ -5572,16 +5596,6 @@ StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
55725596
return getCorrectStaticSpellingForDecl(this);
55735597
}
55745598

5575-
bool FuncDecl::isExplicitNonMutating() const {
5576-
if (auto accessor = dyn_cast<AccessorDecl>(this)) {
5577-
return !isMutating() &&
5578-
!accessor->isGetter() &&
5579-
isInstanceMember() &&
5580-
!getDeclContext()->getDeclaredInterfaceType()->hasReferenceSemantics();
5581-
}
5582-
return false;
5583-
}
5584-
55855599
Type FuncDecl::getResultInterfaceType() const {
55865600
if (!hasInterfaceType())
55875601
return nullptr;

‎test/ModuleInterface/final.swift renamed to ‎test/ModuleInterface/modifiers.swift

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -emit-interface-path %t/Test.swiftinterface -module-name Test %s
2+
// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -emit-interface-path %t/Test.swiftinterface -module-name Test -disable-objc-attr-requires-foundation-module -enable-objc-interop %s
33
// RUN: %FileCheck %s < %t/Test.swiftinterface
4-
// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -disable-objc-attr-requires-foundation-module -emit-interface-path - -module-name Test | %FileCheck %s
4+
// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -disable-objc-attr-requires-foundation-module -emit-interface-path - -module-name Test -enable-objc-interop | %FileCheck %s
55

66
// CHECK: final public class FinalClass {
77
public final class FinalClass {
@@ -40,4 +40,28 @@ public final class FinalClass {
4040
}
4141
@inlinable set {}
4242
}
43+
44+
// CHECK: @objc dynamic final public var d: [[INT]] {
45+
// CHECK-NEXT: {{^}} @objc get{{$}}
46+
// CHECK-NEXT: {{^}} @objc set[[NEWVALUE]]{{$}}
47+
// CHECK-NEXT: }
48+
@objc public dynamic var d: Int {
49+
get {
50+
return 0
51+
}
52+
set {}
53+
}
54+
}
55+
56+
// CHECK: public struct MyStruct {
57+
public struct MyStruct {
58+
// CHECK: public var e: [[INT]] {
59+
// CHECK-NEXT: {{^}} mutating get{{$}}
60+
// CHECK-NEXT: {{^}} @inlinable nonmutating set[[NEWVALUE]] {}
61+
// CHECK-NEXT: }
62+
public var e: Int {
63+
mutating get { return 0 }
64+
@inlinable nonmutating set {}
65+
}
66+
// CHECK-NEXT: }
4367
}

‎test/SILGen/dynamic_accessors.swift

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -o /dev/null -disable-objc-attr-requires-foundation-module -enable-objc-interop -emit-interface-path %t/dynamic_accessors.swiftinterface %s
3+
// RUN: %target-swift-emit-silgen -disable-objc-attr-requires-foundation-module -enable-objc-interop %s | %FileCheck %s
4+
// RUN: %target-swift-frontend -emit-silgen -disable-objc-attr-requires-foundation-module -enable-objc-interop %t/dynamic_accessors.swiftinterface | %FileCheck %s
5+
6+
public class MyObjCClass {
7+
@objc public dynamic var a: Int {
8+
// CHECK-LABEL: sil [thunk] @$s17dynamic_accessors11MyObjCClassC1aSivgTo : $@convention(objc_method) (MyObjCClass) -> Int {
9+
// CHECK: function_ref @$s17dynamic_accessors11MyObjCClassC1aSivg
10+
// CHECK: }
11+
get { return 4 }
12+
13+
// CHECK-LABEL sil [thunk] @$s17dynamic_accessors11MyObjCClassC1aSivsTo : $@convention(objc_method) (Int, MyObjCClass) -> () {
14+
// CHECK: function_ref @$s17dynamic_accessors11MyObjCClassC1aSivs
15+
// CHECK: }
16+
set {}
17+
}
18+
19+
@objc public dynamic subscript(x: Int) -> Int {
20+
// CHECK-LABEL: sil [thunk] @$s17dynamic_accessors11MyObjCClassCyS2icigTo : $@convention(objc_method) (Int, MyObjCClass) -> Int {
21+
// CHECK: function_ref @$s17dynamic_accessors11MyObjCClassCyS2icig
22+
// CHECK: }
23+
get { return x }
24+
25+
// CHECK-LABEL: sil [thunk] @$s17dynamic_accessors11MyObjCClassCyS2icisTo : $@convention(objc_method) (Int, Int, MyObjCClass) -> () {
26+
// CHECK: function_ref @$s17dynamic_accessors11MyObjCClassCyS2icis
27+
// CHECK: }
28+
set {}
29+
}
30+
31+
public init() {}
32+
}
33+
34+
@inlinable public func foo() {
35+
let x = MyObjCClass()
36+
// CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.a!getter.1.foreign
37+
// CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.a!setter.1.foreign
38+
x.a = x.a + 1
39+
40+
// CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.subscript!getter.1.foreign
41+
// CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.subscript!setter.1.foreign
42+
x[4] = x[5]
43+
}

‎test/attr/attr_dynamic_infer.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@ extension Super {
1212

1313
// CHECK: @objc dynamic var prop: Super
1414
@objc var prop: Super {
15-
// CHECK: @objc dynamic get
15+
// CHECK: @objc get
1616
get { return Super() }
17-
// CHECK: @objc dynamic set
17+
// CHECK: @objc set
1818
set { }
1919
}
2020

2121
// CHECK: @objc dynamic subscript(sup: Super) -> Super
2222
@objc subscript(sup: Super) -> Super {
23-
// CHECK: @objc dynamic get
23+
// CHECK: @objc get
2424
get { return sup }
25-
// CHECK: @objc dynamic set
25+
// CHECK: @objc set
2626
set { }
2727
}
2828
}
@@ -37,17 +37,17 @@ extension Sub {
3737

3838
// CHECK: @objc override dynamic var prop: Super
3939
override var prop: Super {
40-
// CHECK: @objc dynamic get
40+
// CHECK: @objc get
4141
get { return Super() }
42-
// CHECK: @objc dynamic set
42+
// CHECK: @objc set
4343
set { }
4444
}
4545

4646
// CHECK: @objc override dynamic subscript(sup: Super) -> Super
4747
override subscript(sup: Super) -> Super {
48-
// CHECK: @objc dynamic get
48+
// CHECK: @objc get
4949
get { return sup }
50-
// CHECK: @objc dynamic set
50+
// CHECK: @objc set
5151
set { }
5252
}
5353

0 commit comments

Comments
 (0)