AST: support @_effects attribute with custom strings.

eeckstein · eeckstein · commit beb2bd2a96b5 · 2022-01-25T11:29:23.000+01:00
In addition to the predefined cases,  like "readnone", "readonly", etc. support providing a custom string, which will be parsed later.
Also, allow multiple effects attributes to be put onto a function.
diff --git a/include/swift/AST/Attr.def b/include/swift/AST/Attr.def
@@ -323,7 +323,7 @@ CONTEXTUAL_DECL_ATTR(weak, ReferenceOwnership,
 CONTEXTUAL_DECL_ATTR_ALIAS(unowned, ReferenceOwnership)
 DECL_ATTR(_effects, Effects,
   OnAbstractFunction |
-  UserInaccessible |
+  AllowMultipleAttributes | UserInaccessible |
   ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,
   50)
 DECL_ATTR(__objc_bridged, ObjCBridged,
diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h
@@ -1227,15 +1227,37 @@ class ExclusivityAttr : public DeclAttribute {
 
 /// Represents the side effects attribute.
 class EffectsAttr : public DeclAttribute {
+  StringRef customString;
+  SourceLoc customStringLoc;
+
 public:
   EffectsAttr(SourceLoc atLoc, SourceRange range, EffectsKind kind)
       : DeclAttribute(DAK_Effects, atLoc, range, /*Implicit=*/false) {
     Bits.EffectsAttr.kind = unsigned(kind);
   }
 
+  EffectsAttr(SourceLoc atLoc, SourceRange range, StringRef customString,
+              SourceLoc customStringLoc)
+      : DeclAttribute(DAK_Effects, atLoc, range, /*Implicit=*/false),
+        customString(customString), customStringLoc(customStringLoc) {
+    Bits.EffectsAttr.kind = unsigned(EffectsKind::Custom);
+  }
+
   EffectsAttr(EffectsKind kind)
   : EffectsAttr(SourceLoc(), SourceRange(), kind) {}
 
+  EffectsAttr(StringRef customString)
+  : EffectsAttr(SourceLoc(), SourceRange(), customString, SourceLoc()) {}
+
+  StringRef getCustomString() const {
+    assert(getKind() == EffectsKind::Custom);
+    return customString;
+  }
+  
+  SourceLoc getCustomStringLocation() const {
+    return customStringLoc;
+  }
+
   EffectsKind getKind() const { return EffectsKind(Bits.EffectsAttr.kind); }
   static bool classof(const DeclAttribute *DA) {
     return DA->getKind() == DAK_Effects;
diff --git a/include/swift/AST/AttrKind.h b/include/swift/AST/AttrKind.h
@@ -89,6 +89,7 @@ enum class EffectsKind : uint8_t {
   ReleaseNone,
   ReadWrite,
   Unspecified,
+  Custom,
   Last_EffectsKind = Unspecified
 };
 
diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def
@@ -1411,6 +1411,11 @@ ERROR(mutually_exclusive_attrs,none,
 ERROR(invalid_infix_on_func,none,
       "'infix' modifier is not required or allowed on func declarations", ())
 
+ERROR(error_in_effects_attribute,none,
+      "error in effects attribute: %0", (StringRef))
+WARNING(warning_in_effects_attribute,none,
+      "effects attribute ignored: %0", (StringRef))
+
 ERROR(expected_in_attribute_list,none,
       "expected ']' or ',' in attribute list", ())
 
diff --git a/include/swift/SIL/SILDeclRef.h b/include/swift/SIL/SILDeclRef.h
@@ -42,6 +42,7 @@ namespace swift {
   class AutoClosureExpr;
   class ASTContext;
   class ClassDecl;
+  class EffectsAttr;
   class FileUnit;
   class SILFunctionType;
   enum IsSerialized_t : unsigned char;
@@ -344,12 +345,6 @@ struct SILDeclRef {
   /// True if the function has __always inline attribute.
   bool isAlwaysInline() const;
   
-  /// \return True if the function has an effects attribute.
-  bool hasEffectsAttribute() const;
-
-  /// \return the effects kind of the function.
-  EffectsKind getEffectsAttribute() const;
-
   /// Return the expected linkage of this declaration.
   SILLinkage getLinkage(ForDefinition_t forDefinition) const;
 
diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp
@@ -885,7 +885,11 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
   case DAK_Optimize:
   case DAK_Exclusivity:
   case DAK_NonSendable:
-    if (DeclAttribute::isDeclModifier(getKind())) {
+    if (getKind() == DAK_Effects &&
+        cast<EffectsAttr>(this)->getKind() == EffectsKind::Custom) {
+      Printer.printAttrName("@_effects");
+      Printer << "(" << cast<EffectsAttr>(this)->getCustomString() << ")";
+    } else if (DeclAttribute::isDeclModifier(getKind())) {
       Printer.printKeyword(getAttrName(), Options);
     } else if (Options.IsForSwiftInterface && getKind() == DAK_ResultBuilder) {
       // Use @_functionBuilder in Swift interfaces to maintain backward
@@ -1312,6 +1316,8 @@ StringRef DeclAttribute::getAttrName() const {
         return "_effects(readwrite)";
       case EffectsKind::Unspecified:
         return "_effects(unspecified)";
+      case EffectsKind::Custom:
+        return "_effects";
     }
   case DAK_AccessControl:
   case DAK_SetterAccess: {
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -1949,19 +1949,58 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
     break;
 
   case DAK_Effects: {
-    auto kind = parseSingleAttrOption<EffectsKind>
-                         (*this, Loc, AttrRange, AttrName, DK)
-                    .when("readonly", EffectsKind::ReadOnly)
-                    .when("readnone", EffectsKind::ReadNone)
-                    .when("readwrite", EffectsKind::ReadWrite)
-                    .when("releasenone", EffectsKind::ReleaseNone)
-                    .diagnoseWhenOmitted();
-    if (!kind)
-      return false;
+    if (!consumeIf(tok::l_paren)) {
+      diagnose(Loc, diag::attr_expected_lparen, AttrName,
+               DeclAttribute::isDeclModifier(DK));      return false;
+    }
+    EffectsKind kind = EffectsKind::Unspecified;
+    SourceLoc customStart, customEnd;
+    {
+      SyntaxParsingContext TokListContext(SyntaxContext, SyntaxKind::TokenList);
 
-    if (!DiscardAttribute)
-      Attributes.add(new (Context) EffectsAttr(AtLoc, AttrRange, *kind));
+      if (Tok.isNot(tok::identifier)) {
+        diagnose(Loc, diag::error_in_effects_attribute, "expected identifier");
+        return false;
+      }
 
+      if (Tok.getText() == "readonly")
+        kind = EffectsKind::ReadOnly;
+      else if (Tok.getText() == "readnone")
+        kind = EffectsKind::ReadNone;
+      else if (Tok.getText() == "readwrite")
+        kind = EffectsKind::ReadWrite;
+      else if (Tok.getText() == "releasenone")
+        kind = EffectsKind::ReleaseNone;
+      else {
+        customStart = customEnd = Tok.getLoc();
+        while (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eof)) {
+          consumeToken();
+        }
+        customEnd = Tok.getLoc();
+        kind = EffectsKind::Custom;
+        AttrRange = SourceRange(Loc, customEnd);
+      }
+      if (kind != EffectsKind::Custom) {
+        AttrRange = SourceRange(Loc, Tok.getRange().getStart());
+        consumeToken(tok::identifier);
+      }
+    }
+    if (!consumeIf(tok::r_paren)) {
+      diagnose(Loc, diag::attr_expected_rparen, AttrName,
+               DeclAttribute::isDeclModifier(DK));
+      return false;
+    }
+
+    if (!DiscardAttribute) {
+      if (kind == EffectsKind::Custom) {
+        StringRef customStr = SourceMgr.extractText(
+                          CharSourceRange(SourceMgr, customStart, customEnd));
+        Attributes.add(new (Context) EffectsAttr(AtLoc, AttrRange,
+                                                 customStr, customStart));
+      } else {
+        Attributes.add(new (Context) EffectsAttr(AtLoc, AttrRange, kind));
+      }
+    }
     break;
   }
 
diff --git a/lib/SIL/IR/SILDeclRef.cpp b/lib/SIL/IR/SILDeclRef.cpp
@@ -727,18 +727,6 @@ bool SILDeclRef::isAlwaysInline() const {
   return false;
 }
 
-bool SILDeclRef::hasEffectsAttribute() const {
-  if (!hasDecl())
-    return false;
-  return getDecl()->getAttrs().hasAttribute<EffectsAttr>();
-}
-
-EffectsKind SILDeclRef::getEffectsAttribute() const {
-  assert(hasEffectsAttribute());
-  EffectsAttr *MA = getDecl()->getAttrs().getAttribute<EffectsAttr>();
-  return MA->getKind();
-}
-
 bool SILDeclRef::isAnyThunk() const {
   return isForeignToNativeThunk() ||
     isNativeToForeignThunk() ||
diff --git a/lib/SIL/IR/SILFunctionBuilder.cpp b/lib/SIL/IR/SILFunctionBuilder.cpp
@@ -13,6 +13,7 @@
 #include "swift/SIL/SILFunctionBuilder.h"
 #include "swift/AST/AttrKind.h"
 #include "swift/AST/Availability.h"
+#include "swift/AST/DiagnosticsParse.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/SemanticAttrs.h"
 
@@ -98,6 +99,24 @@ void SILFunctionBuilder::addFunctionAttributes(
     }
   }
 
+  llvm::SmallVector<const EffectsAttr *, 8> customEffects;
+  for (auto *attr : Attrs.getAttributes<EffectsAttr>()) {
+    auto *effectsAttr = cast<EffectsAttr>(attr);
+    if (effectsAttr->getKind() == EffectsKind::Custom) {
+      customEffects.push_back(effectsAttr);
+    } else {
+      if (F->getEffectsKind() != EffectsKind::Unspecified &&
+          F->getEffectsKind() != effectsAttr->getKind()) {
+        mod.getASTContext().Diags.diagnose(effectsAttr->getLocation(),
+            diag::warning_in_effects_attribute, "mismatching function effects");
+      } else {
+        F->setEffectsKind(effectsAttr->getKind());
+      }
+    }
+  }
+
+  // TODO: handle custom effects.
+
   if (auto *OA = Attrs.getAttribute<OptimizeAttr>()) {
     F->setOptimizationMode(OA->getMode());
   }
@@ -213,10 +232,6 @@ SILFunction *SILFunctionBuilder::getOrCreateFunction(
       constant.isTransparent() ? IsTransparent : IsNotTransparent;
   IsSerialized_t IsSer = constant.isSerialized();
 
-  EffectsKind EK = constant.hasEffectsAttribute()
-                       ? constant.getEffectsAttribute()
-                       : EffectsKind::Unspecified;
-
   Inline_t inlineStrategy = InlineDefault;
   if (constant.isNoinline())
     inlineStrategy = NoInline;
@@ -240,7 +255,7 @@ SILFunction *SILFunctionBuilder::getOrCreateFunction(
                                 IsNotBare, IsTrans, IsSer, entryCount, IsDyn,
                                 IsDistributed, IsNotExactSelfClass,
                                 IsNotThunk, constant.getSubclassScope(),
-                                inlineStrategy, EK);
+                                inlineStrategy);
   F->setDebugScope(new (mod) SILDebugScope(loc, F));
 
   if (constant.isGlobal())
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
@@ -4426,9 +4426,15 @@ llvm::Error DeclDeserializer::deserializeDeclCommon() {
 
       case decls_block::Effects_DECL_ATTR: {
         unsigned kind;
-        serialization::decls_block::EffectsDeclAttrLayout::readRecord(scratch,
-                                                                      kind);
-        Attr = new (ctx) EffectsAttr((EffectsKind)kind);
+        IdentifierID customStringID;
+        serialization::decls_block::EffectsDeclAttrLayout::
+          readRecord(scratch, kind, customStringID);
+        if (customStringID) {
+          assert((EffectsKind)kind == EffectsKind::Custom);
+          Attr = new (ctx) EffectsAttr(MF.getIdentifier(customStringID).str());
+        } else {
+          Attr = new (ctx) EffectsAttr((EffectsKind)kind);
+        }
         break;
       }
       case decls_block::OriginallyDefinedIn_DECL_ATTR: {
diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h
@@ -56,7 +56,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 661; // @exclusivity attribute
+const uint16_t SWIFTMODULE_VERSION_MINOR = 662; // Custom EffectAttrs
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1837,7 +1837,8 @@ namespace decls_block {
 
   using EffectsDeclAttrLayout = BCRecordLayout<
     Effects_DECL_ATTR,
-    BCFixed<2>  // modref value
+    BCFixed<3>,   // EffectKind
+    DeclIDField   // Custom effect string or 0.
   >;
 
   using ForeignErrorConventionLayout = BCRecordLayout<
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
@@ -2531,8 +2531,13 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     case DAK_Effects: {
       auto *theAttr = cast<EffectsAttr>(DA);
       auto abbrCode = S.DeclTypeAbbrCodes[EffectsDeclAttrLayout::Code];
+      IdentifierID customStringID = 0;
+      if (theAttr->getKind() == EffectsKind::Custom) {
+        customStringID = S.addUniquedStringRef(theAttr->getCustomString());
+      }
       EffectsDeclAttrLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
-                                       (unsigned)theAttr->getKind());
+                                        (unsigned)theAttr->getKind(),
+                                        customStringID);
       return;
     }
 
diff --git a/test/ModuleInterface/attrs.swift b/test/ModuleInterface/attrs.swift
@@ -7,6 +7,9 @@
 // CHECK: @_effects(readnone) public func illiterate(){{$}}
 @_effects(readnone) public func illiterate() {}
 
+// CHECK: @_effects(notEscaping arg1.**) public func escapeEffects(arg1: Swift.Int){{$}}
+@_effects(notEscaping arg1.**) public func escapeEffects(arg1: Int) {}
+
 // CHECK-LABEL: @frozen public struct Point {
 @frozen public struct Point {
   // CHECK-NEXT: public var x: Swift.Int
