Merge pull request #41140 from tshortli/warn-on-private-originally-defined-in

[Sema] Warn if the `@_originallyDefinedIn` attribute is applied to a non-public decl

Author: tshortli

include/swift/AST/DiagnosticsSema.def

@@ -1607,6 +1607,11 @@ ERROR(originally_definedin_need_available,none,
 ERROR(originally_definedin_must_not_before_available_version,none,
       "symbols are moved to the current module before they were available in the OSs", (StringRef))
 
+WARNING(originally_defined_in_on_non_public,
+       none, "@%0 does not have any effect on "
+       "%select{private|fileprivate|internal|%error|%error}1 declarations",
+       (StringRef, AccessLevel))
+
 // Alignment attribute
 ERROR(alignment_not_power_of_two,none,
       "alignment value must be a power of two", ())

lib/Sema/TypeCheckAttr.cpp

@@ -3413,6 +3413,22 @@ void AttributeChecker::checkOriginalDefinedInAttrs(Decl *D,
   // 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 (auto *VD = dyn_cast<ValueDecl>(D)) {
+      // This attribute does not make sense on private declarations since
+      // clients can't use them.
+      auto access =
+        VD->getFormalAccessScope(/*useDC=*/nullptr,
+                                 /*treatUsableFromInlineAsPublic=*/true);
+      if (!access.isPublic()) {
+        diagnoseAndRemoveAttr(Attr,
+                              diag::originally_defined_in_on_non_public,
+                              AttrName, VD->getFormalAccess());
+        continue;
+      }
+    }
+    
     if (!Attr->isActivePlatform(Ctx))
       continue;
     auto AtLoc = Attr->AtLoc;
@@ -3425,7 +3441,6 @@ void AttributeChecker::checkOriginalDefinedInAttrs(Decl *D,
                platformString(Platform));
       return;
     }
-    static StringRef AttrName = "_originallyDefinedIn";
     if (!D->getDeclContext()->isModuleScopeContext()) {
       diagnose(AtLoc, diag::originally_definedin_topleve_decl, AttrName);
       return;

test/ModuleInterface/originally-defined-attr.swift

@@ -42,3 +42,9 @@ public struct SimpleThingInAlphabeticalOrderForMacros1 {}
 @available(tvOS 0.7, OSX 1.1, iOS 2.1, watchOS 3.2, *)
 @_originallyDefinedIn(module: "other2", _myProject 1.0)
 public struct SimpleThingInAlphabeticalOrderForMacros2 {}
+
+// CHECK: @_originallyDefinedIn(module: "another", macOS 13.13)
+@available(OSX 10.8, *)
+@_originallyDefinedIn(module: "another", OSX 13.13)
+@usableFromInline
+internal struct SimpleThingInAlphabeticalOrderForMacros3_UsableFromInline {}

test/Parse/original_defined_in_attr.swift

@@ -2,31 +2,46 @@
 // REQUIRES: OS=macosx
 
 @_originallyDefinedIn(module: "foo", OSX 13.13) // expected-error {{need @available attribute for @_originallyDefinedIn}}
-func foo() {}
+public func foo() {}
 
 @_originallyDefinedIn(modulename: "foo", OSX 13.13) // expected-error {{expected 'module: "original"' in the first argument to @_originallyDefinedIn}}
-func foo1() {}
+public func foo1() {}
 
 @_originallyDefinedIn(module: "foo", OSX 13.13.3) // expected-warning {{@_originallyDefinedIn only uses major and minor version number}} expected-error {{need @available attribute for @_originallyDefinedIn}}
-class ToplevelClass {}
+public class ToplevelClass {}
 
 @_originallyDefinedIn(module: "foo") // expected-error {{expected at least one platform version in @_originallyDefinedIn}}
-class ToplevelClass1 {}
+public class ToplevelClass1 {}
 
 @_originallyDefinedIn(OSX 13.13.3) // expected-error {{expected 'module: "original"' in the first argument to @_originallyDefinedIn}}
-class ToplevelClass2 {}
+public class ToplevelClass2 {}
 
 @_originallyDefinedIn(module: "foo", // expected-error {{expected at least one platform version in @_originallyDefinedIn}}
-class ToplevelClass3 {}
+public class ToplevelClass3 {}
 
 @available(OSX 13.10, *)
 @_originallyDefinedIn(module: "foo", * 13.13) // expected-warning {{* as platform name has no effect}} expected-error {{expected at least one platform version in @_originallyDefinedIn}}
 @_originallyDefinedIn(module: "foo", OSX 13.13, iOS 7.0)
 @_originallyDefinedIn(module: "foo", OSX 13.14, * 7.0) // expected-warning {{* as platform name has no effect}} expected-error {{duplicate version number for platform macOS}}
-class ToplevelClass4 {
+public class ToplevelClass4 {
 	@_originallyDefinedIn(module: "foo", OSX 13.13) // expected-error {{'@_originallyDefinedIn' attribute cannot be applied to this declaration}}
 	subscript(index: Int) -> Int {
         get { return 1 }
         set(newValue) {}
 	}
 }
+
+@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}}
+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}}
+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}}
+fileprivate class ToplevelClass7 {}