[cxx-interop] Add diagnostics for nonmutating attr that has no effect

Author: skrtks

include/swift/AST/DiagnosticsClangImporter.def

@@ -109,6 +109,12 @@ WARNING(import_multiple_mainactor_attr,none,
 WARNING(contradicting_mutation_attrs,none,
         "attribute 'nonmutating' is ignored when combined with attribute 'mutating'", ())
 
+WARNING(nonmutating_without_const,none,
+        "attribute 'nonmutating' has no effect on non-const method", ())
+
+WARNING(nonmutating_without_mutable_fields,none,
+        "attribute 'nonmutating' has no effect without any mutable fields", ())
+
 ERROR(module_map_not_found, none, "module map file '%0' not found", (StringRef))
 
 NOTE(macro_not_imported_unsupported_operator, none, "operator not supported in macro arithmetic", ())

lib/ClangImporter/ImportDecl.cpp

@@ -8826,6 +8826,22 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
       }
 
       if (isMutabilityAttr(swiftAttr)) {
+
+        // Check if 'nonmutating' attr is applicable
+        if (swiftAttr->getAttribute() == "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);
+            }
+          }
+        }
+
+        // Check for contradicting mutability attr
         if (seenMutabilityAttr) {
           StringRef seenAttribute =
               seenMutabilityAttr.getValue()->getAttribute();

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

@@ -35,4 +35,17 @@ struct HasMutableProperty {
   }
 };
 
+struct NoMutableProperty {
+  int a;
+
+  // expected-warning@+1 {{attribute 'nonmutating' has no effect without any mutable fields}}
+  int isConst() const __attribute__((__swift_attr__("nonmutating"))) {
+    return a;
+  }
+
+  // expected-warning@+2 {{attribute 'nonmutating' has no effect without any mutable fields}}
+  // expected-warning@+1 {{attribute 'nonmutating' has no effect on non-const method}}
+  int nonConst() __attribute__((__swift_attr__("nonmutating"))) { return a; }
+};
+
 #endif // TEST_INTEROP_CXX_CLASS_INPUTS_MUTABILITY_ANNOTATIONS_H

test/Interop/Cxx/class/mutability-annotations-module-interface.swift

@@ -14,3 +14,9 @@
 // CHECK:   var a: Int32
 // CHECK:   var b: Int32
 // CHECK: }
+
+// CHECK: struct NoMutableProperty {
+// CHECK:   func isConst() -> Int32
+// CHECK:   mutating func nonConst() -> Int32
+// CHECK:   var a: Int32
+// CHECK: }

test/Interop/Cxx/class/mutability-annotations-typechecker.swift

@@ -13,3 +13,7 @@ let _ = objWMutableProperty.annotatedNonMutating()
 let _ = objWMutableProperty.noAnnotation() // expected-error {{cannot use mutating member on immutable value: 'objWMutableProperty' is a 'let' constant}}
 let _ = objWMutableProperty.contradictingAnnotations() // expected-error {{cannot use mutating member on immutable value: 'objWMutableProperty' is a 'let' constant}}
 let _ = objWMutableProperty.duplicateAnnotations()
+
+let objWithoutMutableProperty = NoMutableProperty(a: 42) // expected-note {{change 'let' to 'var' to make it mutable}}
+let _ = objWithoutMutableProperty.isConst()
+let _ = objWithoutMutableProperty.nonConst() // expected-error {{cannot use mutating member on immutable value: 'objWithoutMutableProperty' is a 'let' constant}}