[InterfaceGen] Only print 'mutating' and 'nonmutating' on accessors

include/swift/AST/Decl.h

@@ -5471,10 +5471,6 @@ class FuncDecl : public AbstractFunctionDecl {
     Bits.FuncDecl.SelfAccess = static_cast<unsigned>(mod);
   }
 
-  /// \returns true if this is non-mutating due to applying a 'mutating'
-  /// attribute. For example a "mutating set" accessor.
-  bool isExplicitNonMutating() const;
-
   SourceLoc getStaticLoc() const { return StaticLoc; }
   SourceLoc getFuncLoc() const { return FuncLoc; }
 
@@ -5682,6 +5678,13 @@ class AccessorDecl final : public FuncDecl {
     llvm_unreachable("bad accessor kind");
   }
 
+  /// \returns true if this is non-mutating due to applying a 'mutating'
+  /// attribute. For example a "mutating set" accessor.
+  bool isExplicitNonMutating() const;
+
+  /// Is the accesor one of the kinds that's assumed nonmutating by default?
+  bool isAssumedNonMutating() const;
+
   /// Is this accessor one of the kinds that's implicitly a coroutine?
   bool isCoroutine() const {
     switch (getAccessorKind()) {

lib/AST/ASTPrinter.cpp

@@ -659,6 +659,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
   void printAttributes(const Decl *D);
   void printTypedPattern(const TypedPattern *TP);
   void printBraceStmt(const BraceStmt *stmt, bool newlineIfEmpty = true);
+  void printAccessorDecl(const AccessorDecl *decl);
 
 public:
   void printPattern(const Pattern *pattern);
@@ -692,7 +693,8 @@ class PrintAST : public ASTVisitor<PrintAST> {
   bool shouldPrint(const Decl *D, bool Notify = false);
   bool shouldPrintPattern(const Pattern *P);
   void printPatternType(const Pattern *P);
-  void printAccessors(AbstractStorageDecl *ASD);
+  void printAccessors(const AbstractStorageDecl *ASD);
+  void printMutatingModifiersIfNeeded(const AccessorDecl *accessor);
   void printMembersOfDecl(Decl * NTD, bool needComma = false,
                           bool openBracket = true, bool closeBracket = true);
   void printMembers(ArrayRef<Decl *> members, bool needComma = false,
@@ -813,6 +815,16 @@ static StaticSpellingKind getCorrectStaticSpelling(const Decl *D) {
   }
 }
 
+static bool hasMutatingGetter(const AbstractStorageDecl *ASD) {
+  return ASD->getGetter() && ASD->isGetterMutating();
+}
+
+static bool hasNonMutatingSetter(const AbstractStorageDecl *ASD) {
+  if (!ASD->isSettable(nullptr)) return false;
+  auto setter = ASD->getSetter();
+  return setter && setter->isExplicitNonMutating();
+}
+
 void PrintAST::printAttributes(const Decl *D) {
   if (Options.SkipAttributes)
     return;
@@ -825,9 +837,14 @@ void PrintAST::printAttributes(const Decl *D) {
     Options.ExcludeAttrList.push_back(DAK_Final);
   }
 
-  // Don't print 'final' on accessors.
+  // Don't print any contextual decl modifiers.
+  // We will handle 'mutating' and 'nonmutating' separately.
   if (Options.PrintImplicitAttrs && isa<AccessorDecl>(D)) {
-    Options.ExcludeAttrList.push_back(DAK_Final);
+#define EXCLUDE_ATTR(Class) Options.ExcludeAttrList.push_back(DAK_##Class);
+#define CONTEXTUAL_DECL_ATTR(X, Class, Y, Z) EXCLUDE_ATTR(Class)
+#define CONTEXTUAL_SIMPLE_DECL_ATTR(X, Class, Y, Z) EXCLUDE_ATTR(Class)
+#define CONTEXTUAL_DECL_ATTR_ALIAS(X, Class) EXCLUDE_ATTR(Class)
+#include "swift/AST/Attr.def"
   }
 
   // If the declaration is implicitly @objc, print the attribute now.
@@ -842,6 +859,12 @@ void PrintAST::printAttributes(const Decl *D) {
 
   D->getAttrs().print(Printer, Options, D);
 
+  // Explicitly print 'mutating' and 'nonmutating' before getters and setters
+  // for which that is true.
+  if (auto accessor = dyn_cast<AccessorDecl>(D)) {
+    printMutatingModifiersIfNeeded(accessor);
+  }
+
   Options.ExcludeAttrList.resize(originalExcludeAttrCount);
 }
 
@@ -1514,23 +1537,6 @@ void PrintAST::printBodyIfNecessary(const AbstractFunctionDecl *decl) {
   printBraceStmt(decl->getBody(), /*newlineIfEmpty*/!isa<AccessorDecl>(decl));
 }
 
-static bool isAccessorAssumedNonMutating(AccessorKind kind) {
-  switch (kind) {
-  case AccessorKind::Get:
-  case AccessorKind::Address:
-  case AccessorKind::Read:
-    return true;
-
-  case AccessorKind::Set:
-  case AccessorKind::WillSet:
-  case AccessorKind::DidSet:
-  case AccessorKind::MutableAddress:
-  case AccessorKind::Modify:
-    return false;
-  }
-  llvm_unreachable("bad addressor kind");
-}
-
 static StringRef getAccessorLabel(AccessorDecl *accessor) {
   switch (accessor->getAccessorKind()) {
 #define SINGLETON_ACCESSOR(ID, KEYWORD) \
@@ -1561,7 +1567,17 @@ static StringRef getAccessorLabel(AccessorDecl *accessor) {
   llvm_unreachable("bad accessor kind");
 }
 
-void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
+void PrintAST::printMutatingModifiersIfNeeded(const AccessorDecl *accessor) {
+  if (accessor->isAssumedNonMutating() && accessor->isMutating()) {
+    Printer.printKeyword("mutating");
+    Printer << " ";
+  } else if (accessor->isExplicitNonMutating()) {
+    Printer.printKeyword("nonmutating");
+    Printer << " ";
+  }
+}
+
+void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
   if (isa<VarDecl>(ASD) && !Options.PrintPropertyAccessors)
     return;
 
@@ -1590,13 +1606,9 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
   bool inProtocol = isa<ProtocolDecl>(ASD->getDeclContext());
   if ((inProtocol && !Options.PrintAccessorBodiesInProtocols) ||
       PrintAbstract) {
-    bool mutatingGetter = ASD->getGetter() && ASD->isGetterMutating();
     bool settable = ASD->isSettable(nullptr);
-    bool nonmutatingSetter = false;
-    if (settable && !ASD->isSetterMutating() && ASD->isInstanceMember() &&
-        !ASD->getDeclContext()->getDeclaredInterfaceType()
-            ->hasReferenceSemantics())
-      nonmutatingSetter = true;
+    bool mutatingGetter = hasMutatingGetter(ASD);
+    bool nonmutatingSetter = hasNonMutatingSetter(ASD);
 
     // We're about to print something like this:
     //   { mutating? get (nonmutating? set)? }
@@ -1660,18 +1672,8 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
     if (!Accessor || !shouldPrint(Accessor))
       return true;
     if (!PrintAccessorBody) {
-      if (isAccessorAssumedNonMutating(Accessor->getAccessorKind())) {
-        if (Accessor->isMutating()) {
-          Printer << " ";
-          Printer.printKeyword("mutating");
-        }
-      } else {
-        if (Accessor->isExplicitNonMutating()) {
-          Printer << " ";
-          Printer.printKeyword("nonmutating");
-        }
-      }
       Printer << " ";
+      printMutatingModifiersIfNeeded(Accessor);
       Printer.printKeyword(getAccessorLabel(Accessor));
     } else {
       {

lib/AST/Decl.cpp

@@ -5562,6 +5562,30 @@ AccessorDecl *AccessorDecl::create(ASTContext &ctx,
   return D;
 }
 
+bool AccessorDecl::isAssumedNonMutating() const {
+  switch (getAccessorKind()) {
+  case AccessorKind::Get:
+  case AccessorKind::Address:
+  case AccessorKind::Read:
+    return true;
+
+  case AccessorKind::Set:
+  case AccessorKind::WillSet:
+  case AccessorKind::DidSet:
+  case AccessorKind::MutableAddress:
+  case AccessorKind::Modify:
+    return false;
+  }
+  llvm_unreachable("bad accessor kind");
+}
+
+bool AccessorDecl::isExplicitNonMutating() const {
+  return !isMutating() &&
+    !isAssumedNonMutating() &&
+    isInstanceMember() &&
+    !getDeclContext()->getDeclaredInterfaceType()->hasReferenceSemantics();
+}
+
 StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
   assert(getDeclContext()->isTypeContext());
   if (!isStatic())
@@ -5572,16 +5596,6 @@ StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const {
   return getCorrectStaticSpellingForDecl(this);
 }
 
-bool FuncDecl::isExplicitNonMutating() const {
-  if (auto accessor = dyn_cast<AccessorDecl>(this)) {
-    return !isMutating() &&
-         !accessor->isGetter() &&
-         isInstanceMember() &&
-         !getDeclContext()->getDeclaredInterfaceType()->hasReferenceSemantics();
-  }
-  return false;
-}
-
 Type FuncDecl::getResultInterfaceType() const {
   if (!hasInterfaceType())
     return nullptr;

test/ModuleInterface/final.swift → test/ModuleInterface/modifiers.swift

@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -emit-interface-path %t/Test.swiftinterface -module-name Test %s
+// 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
 // RUN: %FileCheck %s < %t/Test.swiftinterface
-// 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
+// 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
 
 // CHECK: final public class FinalClass {
 public final class FinalClass {
@@ -40,4 +40,28 @@ public final class FinalClass {
     }
     @inlinable set {}
   }
+
+  // CHECK: @objc dynamic final public var d: [[INT]] {
+  // CHECK-NEXT: {{^}} @objc get{{$}}
+  // CHECK-NEXT: {{^}} @objc set[[NEWVALUE]]{{$}}
+  // CHECK-NEXT: }
+  @objc public dynamic var d: Int {
+    get {
+      return 0
+    }
+    set {}
+  }
+}
+
+// CHECK: public struct MyStruct {
+public struct MyStruct {
+  // CHECK: public var e: [[INT]] {
+  // CHECK-NEXT: {{^}} mutating get{{$}}
+  // CHECK-NEXT: {{^}} @inlinable nonmutating set[[NEWVALUE]] {}
+  // CHECK-NEXT: }
+  public var e: Int {
+    mutating get { return 0 }
+    @inlinable nonmutating set {}
+  }
+  // CHECK-NEXT: }
 }

test/SILGen/dynamic_accessors.swift

@@ -0,0 +1,43 @@
+// RUN: %empty-directory(%t)
+// 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
+// RUN: %target-swift-emit-silgen -disable-objc-attr-requires-foundation-module -enable-objc-interop %s | %FileCheck %s
+// RUN: %target-swift-frontend -emit-silgen -disable-objc-attr-requires-foundation-module -enable-objc-interop %t/dynamic_accessors.swiftinterface | %FileCheck %s
+
+public class MyObjCClass {
+  @objc public dynamic var a: Int {
+    // CHECK-LABEL: sil [thunk] @$s17dynamic_accessors11MyObjCClassC1aSivgTo : $@convention(objc_method) (MyObjCClass) -> Int {
+    // CHECK:  function_ref @$s17dynamic_accessors11MyObjCClassC1aSivg
+    // CHECK: }
+    get { return 4 }
+
+    // CHECK-LABEL sil [thunk] @$s17dynamic_accessors11MyObjCClassC1aSivsTo : $@convention(objc_method) (Int, MyObjCClass) -> () {
+    // CHECK: function_ref @$s17dynamic_accessors11MyObjCClassC1aSivs
+    // CHECK: }
+    set {}
+  }
+
+  @objc public dynamic subscript(x: Int) -> Int {
+    // CHECK-LABEL: sil [thunk] @$s17dynamic_accessors11MyObjCClassCyS2icigTo : $@convention(objc_method) (Int, MyObjCClass) -> Int {
+    // CHECK: function_ref @$s17dynamic_accessors11MyObjCClassCyS2icig
+    // CHECK: }
+    get { return x }
+
+    // CHECK-LABEL: sil [thunk] @$s17dynamic_accessors11MyObjCClassCyS2icisTo : $@convention(objc_method) (Int, Int, MyObjCClass) -> () {
+    // CHECK: function_ref @$s17dynamic_accessors11MyObjCClassCyS2icis
+    // CHECK: }
+    set {}
+  }
+
+  public init() {}
+}
+
+@inlinable public func foo() {
+  let x = MyObjCClass()
+  // CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.a!getter.1.foreign
+  // CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.a!setter.1.foreign
+  x.a = x.a + 1
+
+  // CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.subscript!getter.1.foreign
+  // CHECK: objc_method %{{[0-9]+}} : $MyObjCClass, #MyObjCClass.subscript!setter.1.foreign
+  x[4] = x[5]
+}

test/attr/attr_dynamic_infer.swift

@@ -12,17 +12,17 @@ extension Super {
 
   // CHECK: @objc dynamic var prop: Super
   @objc var prop: Super {
-    // CHECK: @objc dynamic get
+    // CHECK: @objc get
     get { return Super() }
-    // CHECK: @objc dynamic set
+    // CHECK: @objc set
     set { }
   }
 
   // CHECK: @objc dynamic subscript(sup: Super) -> Super
   @objc subscript(sup: Super) -> Super {
-    // CHECK: @objc dynamic get
+    // CHECK: @objc get
     get { return sup }
-    // CHECK: @objc dynamic set
+    // CHECK: @objc set
     set { }
   }
 }
@@ -37,17 +37,17 @@ extension Sub {
 
   // CHECK: @objc override dynamic var prop: Super
   override var prop: Super {
-    // CHECK: @objc dynamic get
+    // CHECK: @objc get
     get { return Super() }
-    // CHECK: @objc dynamic set
+    // CHECK: @objc set
     set { }
   }
 
   // CHECK: @objc override dynamic subscript(sup: Super) -> Super
   override subscript(sup: Super) -> Super {
-    // CHECK: @objc dynamic get
+    // CHECK: @objc get
     get { return sup }
-    // CHECK: @objc dynamic set
+    // CHECK: @objc set
     set { }
   }