Merge pull request #80383 from beccadax/abi-inspected-your-appearance-again

More `@abi` checking

Author: beccadax

include/swift/AST/Attr.h

@@ -380,21 +380,14 @@ class DeclAttribute : public AttributeBase {
     /// valid if they match. 
     EquivalentInABIAttr = 1ull << 18,
 
-    /// Attribute can be used in an \c \@abi attribute, but must match
-    /// equivalent on API decl; if omitted, API decl's attribute will be 
-    /// cloned. Use where you would want to use \c EquivalentInABIAttr but 
-    /// repeating the attribute is judged too burdensome.
-    InferredInABIAttr = 1ull << 19,
-
     /// Use for attributes which are \em only valid on declarations that cannot
     /// have an \c @abi attribute, such as \c ImportDecl .
-    UnreachableInABIAttr = 1ull << 20,
+    UnreachableInABIAttr = 1ull << 19,
   };
 
   enum : uint64_t {
     InABIAttrMask = ForbiddenInABIAttr | UnconstrainedInABIAttr
-                  | EquivalentInABIAttr | InferredInABIAttr
-                  | UnreachableInABIAttr
+                  | EquivalentInABIAttr | UnreachableInABIAttr
   };
 
   LLVM_READNONE

include/swift/AST/DeclAttr.def

@@ -143,7 +143,7 @@ SIMPLE_DECL_ATTR(NSManaged, NSManaged,
 
 CONTEXTUAL_SIMPLE_DECL_ATTR(lazy, Lazy,
   OnVar,
-  DeclModifier | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr,
+  DeclModifier | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | UnconstrainedInABIAttr,
   16)
 
 SIMPLE_DECL_ATTR(LLDBDebuggerFunction, LLDBDebuggerFunction,
@@ -163,7 +163,7 @@ SIMPLE_DECL_ATTR(unsafe_no_objc_tagged_pointer, UnsafeNoObjCTaggedPointer,
 
 DECL_ATTR(inline, Inline,
   OnVar | OnSubscript | OnAbstractFunction,
-  ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr,
+  ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   20)
 
 DECL_ATTR(_semantics, Semantics,
@@ -193,7 +193,7 @@ CONTEXTUAL_SIMPLE_DECL_ATTR(postfix, Postfix,
 
 SIMPLE_DECL_ATTR(_transparent, Transparent,
   OnFunc | OnAccessor | OnConstructor | OnVar | OnDestructor,
-  UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr,
+  UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   26)
 
 SIMPLE_DECL_ATTR(requires_stored_property_inits, RequiresStoredPropertyInits,
@@ -216,7 +216,7 @@ SIMPLE_DECL_ATTR(_fixed_layout, FixedLayout,
 
 SIMPLE_DECL_ATTR(inlinable, Inlinable,
   OnVar | OnSubscript | OnAbstractFunction,
-  ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr,
+  ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   32)
 
 DECL_ATTR(_specialize, Specialize,
@@ -456,7 +456,7 @@ DECL_ATTR(_dynamicReplacement, DynamicReplacement,
 
 SIMPLE_DECL_ATTR(_borrowed, Borrowed,
   OnVar | OnSubscript,
-  UserInaccessible | NotSerialized | ABIBreakingToAdd | ABIBreakingToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr,
+  UserInaccessible | NotSerialized | ABIBreakingToAdd | ABIBreakingToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   81)
 
 DECL_ATTR(_private, PrivateImport,
@@ -466,7 +466,7 @@ DECL_ATTR(_private, PrivateImport,
 
 SIMPLE_DECL_ATTR(_alwaysEmitIntoClient, AlwaysEmitIntoClient,
   OnVar | OnSubscript | OnAbstractFunction,
-  UserInaccessible | ABIBreakingToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | InferredInABIAttr,
+  UserInaccessible | ABIBreakingToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   83)
 
 SIMPLE_DECL_ATTR(_implementationOnly, ImplementationOnly,

include/swift/AST/DiagnosticsParse.def

@@ -1571,6 +1571,11 @@ ERROR(attr_unsupported_on_target, none,
 ERROR(attr_name_unsupported_on_target, none,
       "attribute '%0' is unsupported on target '%1'", (StringRef, StringRef))
 
+// abi attribute
+ERROR(attr_abi_incompatible_kind,none,
+      "cannot use %0 in '@abi'",
+      (DescriptiveDeclKind))
+
 // availability
 ERROR(attr_availability_platform,none,
       "expected platform name or '*' for '%0' attribute", (StringRef))

include/swift/AST/DiagnosticsSema.def

@@ -7457,7 +7457,7 @@ ERROR(property_wrapper_effectful,none,
 
 ERROR(property_with_wrapper_conflict_attribute,none,
       "property %0 with a wrapper cannot also be "
-      "%select{lazy|@NSCopying|@NSManaged|weak|unowned|unmanaged}1",
+      "%select{lazy|@NSCopying|@NSManaged|@abi|weak|unowned|unmanaged}1",
       (Identifier, int))
 ERROR(property_wrapper_not_single_var, none,
       "property wrapper can only apply to a single variable", ())
@@ -8467,9 +8467,6 @@ ERROR(attr_abi_extra_attr,none,
 ERROR(attr_abi_forbidden_attr,none,
       "unused '%0' %select{attribute|modifier}1 in '@abi'",
       (StringRef, bool))
-REMARK(abi_attr_inferred_attribute,none,
-       "inferred '%0' in '@abi' to match %select{attribute|modifier}1 on API",
-       (StringRef, bool))
 
 ERROR(attr_abi_mismatched_attr,none,
       "'%0' %select{attribute|modifier}1 in '@abi' should match '%2'",
@@ -8509,6 +8506,13 @@ ERROR(attr_abi_no_default_arguments,none,
       "affect the parameter's ABI",
       (Decl *))
 
+ERROR(attr_abi_no_macros,none,
+      "%kind0 cannot be expanded in '@abi' attribute",
+      (Decl *))
+ERROR(attr_abi_no_lazy,none,
+      "'lazy' is not compatible with '@abi' attribute",
+      ())
+
 // These macros insert 'final', 'non-final', or nothing depending on both the
 // current decl and its counterpart, such that 'non-final' is used if the
 // counterpart would be described as 'final' or 'static'. They must be kept in

include/swift/Basic/LangOptions.h

@@ -269,9 +269,6 @@ namespace swift {
     /// Emit a remark on early exit in explicit interface build
     bool EnableSkipExplicitInterfaceModuleBuildRemarks = false;
 
-    /// Emit a remark when \c \@abi infers an attribute or modifier.
-    bool EnableABIInferenceRemarks = false;
-
     ///
     /// Support for alternate usage modes
     ///

include/swift/Option/Options.td

@@ -468,10 +468,6 @@ def remark_module_serialization : Flag<["-"], "Rmodule-serialization">,
   Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
   HelpText<"Emit remarks about module serialization">;
 
-def remark_abi_inference : Flag<["-"], "Rabi-inference">,
-  Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
-  HelpText<"Emit a remark when an '@abi' attribute adds an attribute or modifier to the ABI declaration based on its presence in the API">;
-
 def emit_tbd : Flag<["-"], "emit-tbd">,
   HelpText<"Emit a TBD file">,
   Flags<[FrontendOption, NoInteractiveOption, SupplementaryOutput]>;

lib/AST/Attr.cpp

@@ -61,7 +61,7 @@ static_assert(IsTriviallyDestructible<DeclAttributes>::value,
                 DeclAttribute::APIBreakingToRemove | DeclAttribute::APIStableToRemove), \
                 #Name " needs to specify either APIBreakingToRemove or APIStableToRemove"); \
   static_assert(DeclAttribute::hasOneBehaviorFor##Id(DeclAttribute::InABIAttrMask), \
-                #Name " needs to specify exactly one of ForbiddenInABIAttr, UnconstrainedInABIAttr, EquivalentInABIAttr, InferredInABIAttr, or UnreachableInABIAttr");
+                #Name " needs to specify exactly one of ForbiddenInABIAttr, UnconstrainedInABIAttr, EquivalentInABIAttr, or UnreachableInABIAttr");
 #include "swift/AST/DeclAttr.def"
 
 #define TYPE_ATTR(_, Id)                                                       \

lib/AST/FeatureSet.cpp

@@ -709,8 +709,12 @@ FeatureSet swift::getUniqueFeaturesUsed(Decl *decl) {
   // Remove all the features used by all enclosing declarations.
   Decl *enclosingDecl = decl;
   while (!features.empty()) {
+    // If we were in an @abi attribute, collect from the API counterpart.
+    auto abiRole = ABIRoleInfo(enclosingDecl);
+    if (!abiRole.providesAPI() && abiRole.getCounterpart())
+      enclosingDecl = abiRole.getCounterpart();
     // Find the next outermost enclosing declaration.
-    if (auto accessor = dyn_cast<AccessorDecl>(enclosingDecl))
+    else if (auto accessor = dyn_cast<AccessorDecl>(enclosingDecl))
       enclosingDecl = accessor->getStorage();
     else
       enclosingDecl = enclosingDecl->getDeclContext()->getAsDecl();

lib/AST/NameLookup.cpp

@@ -2317,6 +2317,13 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
   if (!ObjCMethodLookup && !createObjCMethodLookup())
     return;
 
+  // Only record API decls.
+  Decl *abiRoleDecl = method;
+  if (auto accessor = dyn_cast<AccessorDecl>(method))
+    abiRoleDecl = accessor->getStorage();
+  if (!ABIRoleInfo(abiRoleDecl).providesAPI())
+    return;
+
   // Record the method.
   bool isInstanceMethod = method->isObjCInstanceMethod();
   auto &vec = (*ObjCMethodLookup)[{selector, isInstanceMethod}].Methods;

lib/Frontend/CompilerInvocation.cpp

@@ -1422,8 +1422,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
 
   Opts.EnableSkipExplicitInterfaceModuleBuildRemarks = Args.hasArg(OPT_remark_skip_explicit_interface_build);
 
-  Opts.EnableABIInferenceRemarks = Args.hasArg(OPT_remark_abi_inference);
-
   if (Args.hasArg(OPT_experimental_skip_non_exportable_decls)) {
     // Only allow -experimental-skip-non-exportable-decls if either library
     // evolution is enabled (in which case the module's ABI is independent of

lib/Parse/ParseDecl.cpp

@@ -3359,9 +3359,18 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
     }
 
     if (abiDecl) {
-      Attributes.add(new (Context) ABIAttr(abiDecl,
-                                           AtLoc, { Loc, rParenLoc },
-                                           /*implicit=*/false));
+      auto attr = new (Context) ABIAttr(abiDecl, AtLoc, { Loc, rParenLoc },
+                                        /*implicit=*/false);
+
+      // Diagnose syntactically invalid abiDecl kind here to match behavior of
+      // Swift parser.
+      if (!attr->canAppearOnDecl(abiDecl) && !isa<PatternBindingDecl>(abiDecl)){
+        diagnose(abiDecl->getLoc(), diag::attr_abi_incompatible_kind,
+                 abiDecl->getDescriptiveKind());
+        attr->setInvalid();
+      }
+
+      Attributes.add(attr);
     }
 
     break;

lib/SIL/IR/SILDeclRef.cpp

@@ -883,6 +883,9 @@ SerializedKind_t SILDeclRef::getSerializedKind() const {
 
   auto *d = getDecl();
 
+  ASSERT(ABIRoleInfo(d).providesAPI()
+            && "should not get serialization info from ABI-only decl");
+
   // Default and property wrapper argument generators are serialized if the
   // containing declaration is public.
   if (isDefaultArgGenerator() || (isPropertyWrapperBackingInitializer() &&
@@ -1021,6 +1024,9 @@ bool SILDeclRef::isNoinline() const {
     return false;
 
   auto *decl = getDecl();
+  ASSERT(ABIRoleInfo(decl).providesAPI()
+            && "should not get inline attr from ABI-only decl");
+
   if (auto *attr = decl->getAttrs().getAttribute<InlineAttr>())
     if (attr->getKind() == InlineKind::Never)
       return true;
@@ -1051,6 +1057,9 @@ bool SILDeclRef::isAlwaysInline() const {
     return false;
   }
 
+  ASSERT(ABIRoleInfo(decl).providesAPI()
+            && "should not get inline attr from ABI-only decl");
+
   if (auto attr = decl->getAttrs().getAttribute<InlineAttr>())
     if (attr->getKind() == InlineKind::Always)
       return true;
@@ -1070,6 +1079,10 @@ bool SILDeclRef::isBackDeployed() const {
     return false;
 
   auto *decl = getDecl();
+
+  ASSERT(ABIRoleInfo(decl).providesAPI()
+            && "should not get backDeployed from ABI-only decl");
+
   if (auto afd = dyn_cast<AbstractFunctionDecl>(decl))
     return afd->isBackDeployed(getASTContext());
 
@@ -1198,6 +1211,9 @@ static std::string mangleClangDecl(Decl *decl, bool isForeign) {
 }
 
 std::string SILDeclRef::mangle(ManglingKind MKind) const {
+  ASSERT(!hasDecl() || ABIRoleInfo(getDecl()).providesAPI()
+            && "SILDeclRef mangling ABI decl directly?");
+
   using namespace Mangle;
   ASTMangler mangler(getASTContext());
 
@@ -1268,9 +1284,6 @@ std::string SILDeclRef::mangle(ManglingKind MKind) const {
     if (auto *ACE = getAbstractClosureExpr())
       return mangler.mangleClosureEntity(ACE, SKind);
 
-    ASSERT(ABIRoleInfo(getDecl()).providesAPI()
-              && "SILDeclRef mangling ABI decl directly?");
-
     // As a special case, functions can have manually mangled names.
     // Use the SILGen name only for the original non-thunked, non-curried entry
     // point.

lib/Sema/TypeCheckAttr.cpp

@@ -1110,6 +1110,11 @@ void AttributeChecker::visitLazyAttr(LazyAttr *attr) {
   // are already lazily initialized).
   if (VD->isStatic() || varDC->isModuleScopeContext())
     diagnoseAndRemoveAttr(attr, diag::lazy_on_already_lazy_global);
+
+  // 'lazy' can't be used in or with `@abi` because it has auxiliary decls.
+  auto abiRole = ABIRoleInfo(D);
+  if (!abiRole.providesABI() || !abiRole.providesAPI())
+    diagnoseAndRemoveAttr(attr, diag::attr_abi_no_lazy);
 }
 
 bool AttributeChecker::visitAbstractAccessControlAttr(
@@ -4406,6 +4411,12 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
         }
       }
 
+      // Macros can't be attached to ABI-only decls. (If we diagnosed above,
+      // don't bother with this.)
+      if (attr->isValid() && !ABIRoleInfo(D).providesAPI()) {
+        diagnoseAndRemoveAttr(attr, diag::attr_abi_no_macros, macro);
+      }
+
       return;
     }
 
@@ -4487,16 +4498,25 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
   // function, storage with an explicit getter, or parameter of function type.
   if (nominal->getAttrs().hasAttribute<ResultBuilderAttr>()) {
     ValueDecl *decl;
+    ValueDecl *abiRelevantDecl;
     if (auto param = dyn_cast<ParamDecl>(D)) {
       decl = param;
+      abiRelevantDecl = dyn_cast<ValueDecl>(
+                            param->getDeclContext()->getAsDecl());
     } else if (auto func = dyn_cast<FuncDecl>(D)) {
       decl = func;
+      abiRelevantDecl = func;
     } else if (auto storage = dyn_cast<AbstractStorageDecl>(D)) {
       decl = storage;
+      abiRelevantDecl = storage;
 
       // Check whether this is a storage declaration that is not permitted
       // to have a result builder attached.
       auto shouldDiagnose = [&]() -> bool {
+        // We'll diagnose use in @abi later.
+        if (!ABIRoleInfo(abiRelevantDecl).providesAPI())
+          return false;
+
         // An uninitialized stored property in a struct can have a function
         // builder attached.
         if (auto var = dyn_cast<VarDecl>(decl)) {
@@ -4540,6 +4560,14 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
       return;
     }
 
+    // Result builders shouldn't be applied to an ABI-only decl because they
+    // have no ABI effect.
+    if (!ABIRoleInfo(abiRelevantDecl).providesAPI()) {
+      diagnoseAndRemoveAttr(attr, diag::attr_abi_forbidden_attr,
+                            nominal->getNameStr(), /*isModifier=*/false);
+      return;
+    }
+
     // Diagnose and ignore arguments.
     if (attr->hasArgs()) {
       diagnose(attr->getLocation(), diag::result_builder_arguments)