Merge pull request #42031 from tshortli/originally-defined-in-on-non-public-warn-once

Sema: Warn about `@_originallyDefinedIn` attributes on non-public decls at most once per attribute

Author: tshortli

include/swift/AST/DiagnosticsSema.def

@@ -1603,10 +1603,11 @@ NOTE(option_set_empty_set_init,none,
      "use [] to silence this warning", ())
 
 ERROR(originally_definedin_topleve_decl,none,
-      "@%0 is only applicable to top-level decl", (StringRef))
+      "'%0' is only applicable to top-level decl", (DeclAttribute))
 
 ERROR(originally_definedin_must_not_before_available_version,none,
-      "symbols are moved to the current module before they were available in the OSs", (StringRef))
+      "symbols are moved to the current module before they were available in "
+      "the OSs", ())
 
 // Alignment attribute
 ERROR(alignment_not_power_of_two,none,

lib/Sema/TypeCheckAttr.cpp

@@ -3483,13 +3483,12 @@ void AttributeChecker::checkOriginalDefinedInAttrs(
   // Attrs are in the reverse order of the source order. We need to visit them
   // in source order to diagnose the later attribute.
   for (auto *Attr: Attrs) {
-    static StringRef AttrName = "_originallyDefinedIn";
+    if (!Attr->isActivePlatform(Ctx))
+      continue;
 
     if (diagnoseAndRemoveAttrIfDeclIsNonPublic(Attr, /*isError=*/false))
       continue;
 
-    if (!Attr->isActivePlatform(Ctx))
-      continue;
     auto AtLoc = Attr->AtLoc;
     auto Platform = Attr->Platform;
     if (!seenPlatforms.insert({Platform, AtLoc}).second) {
@@ -3501,7 +3500,7 @@ void AttributeChecker::checkOriginalDefinedInAttrs(
       return;
     }
     if (!D->getDeclContext()->isModuleScopeContext()) {
-      diagnose(AtLoc, diag::originally_definedin_topleve_decl, AttrName);
+      diagnose(AtLoc, diag::originally_definedin_topleve_decl, Attr);
       return;
     }
 
@@ -3511,8 +3510,7 @@ void AttributeChecker::checkOriginalDefinedInAttrs(
     auto IntroVer = D->getIntroducedOSVersion(Platform);
     if (IntroVer.getValue() > Attr->MovedVersion) {
       diagnose(AtLoc,
-               diag::originally_definedin_must_not_before_available_version,
-               AttrName);
+               diag::originally_definedin_must_not_before_available_version);
       return;
     }
   }

test/Parse/original_defined_in_attr.swift

@@ -34,15 +34,19 @@ public class ToplevelClass4 {
 
 @available(OSX 13.10, *)
 @_originallyDefinedIn(module: "foo", OSX 13.13) // expected-warning {{'@_originallyDefinedIn' does not have any effect on internal declarations}}
-@_originallyDefinedIn(module: "foo", iOS 7.0) // expected-warning {{'@_originallyDefinedIn' does not have any effect on internal declarations}}
+@_originallyDefinedIn(module: "foo", iOS 7.0)
 internal class ToplevelClass5 {}
 
 @available(OSX 13.10, *)
 @_originallyDefinedIn(module: "foo", OSX 13.13) // expected-warning {{'@_originallyDefinedIn' does not have any effect on private declarations}}
-@_originallyDefinedIn(module: "foo", iOS 7.0) // expected-warning {{'@_originallyDefinedIn' does not have any effect on private declarations}}
+@_originallyDefinedIn(module: "foo", iOS 7.0)
 private class ToplevelClass6 {}
 
 @available(OSX 13.10, *)
 @_originallyDefinedIn(module: "foo", OSX 13.13) // expected-warning {{'@_originallyDefinedIn' does not have any effect on fileprivate declarations}}
-@_originallyDefinedIn(module: "foo", iOS 7.0) // expected-warning {{'@_originallyDefinedIn' does not have any effect on fileprivate declarations}}
+@_originallyDefinedIn(module: "foo", iOS 7.0)
 fileprivate class ToplevelClass7 {}
+
+@available(OSX 13.10, *)
+@_originallyDefinedIn(module: "foo", OSX 13.13, iOS 7.0) // expected-warning {{'@_originallyDefinedIn' does not have any effect on internal declarations}}
+internal class ToplevelClass8 {}

test/Sema/diag_originally_definedin.swift

@@ -8,7 +8,7 @@ public func foo() {}
 @available(macOS 10.13, *)
 @_originallyDefinedIn(module: "original", OSX 10.12) // expected-error {{symbols are moved to the current module before they were available in the OSs}}
 public class C {
-  @_originallyDefinedIn(module: "original", OSX 10.13) // expected-error {{@_originallyDefinedIn is only applicable to top-level decl}}
+  @_originallyDefinedIn(module: "original", OSX 10.13) // expected-error {{'@_originallyDefinedIn' is only applicable to top-level decl}}
   public func foo() {}
 }