Merge pull request #61500 from compnerd/mutable

ClangImporter: simplify logic for mutability attribute

Author: compnerd

include/swift/AST/DiagnosticsClangImporter.def

@@ -107,7 +107,8 @@ WARNING(import_multiple_mainactor_attr,none,
       (StringRef, StringRef))
 
 WARNING(contradicting_mutation_attrs,none,
-        "attribute 'nonmutating' is ignored when combined with attribute 'mutating'", ())
+        "attribute '%0' is ignored when combined with attribute '%1'",
+        (StringRef, StringRef))
 
 WARNING(nonmutating_without_const,none,
         "attribute 'nonmutating' has no effect on non-const method", ())

lib/ClangImporter/ImportDecl.cpp

@@ -7242,7 +7242,7 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
       ClangDecl = cast<clang::NamedDecl>(maybeDefinition.getValue());
 
   Optional<const clang::SwiftAttrAttr *> seenMainActorAttr;
-  Optional<const clang::SwiftAttrAttr *> seenMutabilityAttr;
+  const clang::SwiftAttrAttr *seenMutabilityAttr = nullptr;
   PatternBindingInitializer *initContext = nullptr;
 
   auto importAttrsFromDecl = [&](const clang::NamedDecl *ClangDecl) {
@@ -7274,14 +7274,16 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
       }
 
       if (isMutabilityAttr(swiftAttr)) {
+        StringRef attr = swiftAttr->getAttribute();
 
         // Check if 'nonmutating' attr is applicable
-        if (swiftAttr->getAttribute() == "nonmutating") {
+        if (attr == "nonmutating") {
           if (auto *method = dyn_cast<clang::CXXMethodDecl>(ClangDecl)) {
             if (!method->isConst()) {
               diagnose(HeaderLoc(swiftAttr->getLocation()),
                        diag::nonmutating_without_const);
             }
+
             if (!method->getParent()->hasMutableFields()) {
               diagnose(HeaderLoc(swiftAttr->getLocation()),
                        diag::nonmutating_without_mutable_fields);
@@ -7291,18 +7293,11 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
 
         // Check for contradicting mutability attr
         if (seenMutabilityAttr) {
-          StringRef seenAttribute =
-              seenMutabilityAttr.getValue()->getAttribute();
-          if ((seenAttribute == "nonmutating" &&
-               swiftAttr->getAttribute() == "mutating") ||
-              (seenAttribute == "mutating" &&
-               swiftAttr->getAttribute() == "nonmutating")) {
-            const clang::SwiftAttrAttr *nonmutatingAttr =
-                seenAttribute == "nonmutating" ? seenMutabilityAttr.getValue()
-                                               : swiftAttr;
-
-            diagnose(HeaderLoc(nonmutatingAttr->getLocation()),
-                     diag::contradicting_mutation_attrs);
+          StringRef previous = seenMutabilityAttr->getAttribute();
+
+          if (previous != attr) {
+            diagnose(HeaderLoc(swiftAttr->getLocation()),
+                     diag::contradicting_mutation_attrs, attr, previous);
             continue;
           }
         }

test/Interop/Cxx/class/Inputs/mutability-annotations.h

@@ -25,7 +25,7 @@ struct HasMutableProperty {
 
   int noAnnotation() const { return b; }
 
-  // expected-warning@+1 {{attribute 'nonmutating' is ignored when combined with attribute 'mutating'}}
+  // expected-warning@+1 {{attribute 'mutating' is ignored when combined with attribute 'nonmutating'}}
   int contradictingAnnotations() const __attribute__((__swift_attr__("nonmutating"))) __attribute__((__swift_attr__("mutating"))) {
     return b;
   }