Requestify the Raw Value Accessor

Make getRawValueExpr() return a checked value.

This entails a strange kind of request that effectively acts like
a cache warmer.  In order to properly check the raw value expression for
a single case, we actually need all the other cases for the
autoincrementing synthesis logic.  The strategy is therefore to have the
request act at the level of the parent EnumDecl and check all the values
at once.  We also cache at the level of the EnumDecl so the cache
"warms" for all enum elements simultaneously.

The request also abuses TypeResolutionStage to act as an indicator for
how much information to compute.  In the minimal case, we will return
a complete accounting of (auto-incremented) raw values.  In the maximal
case we will also check and record types and emit diagnostics.  The
minimal case is uncached to support repeated evaluation.

Note that computing the interface type of an @objc enum decl *must*
force this request.  The enum's raw values are part of the ABI, and we
should not get all the way to IRGen before discovering that we cannot
possibly lay out the enum.  In the future, we might want to consider
moving this check earlier or have IRGen tolerate broken cases but for
now we will maintain the status quo and not have IRGen emit
diagnostics.

Author: CodaFi

include/swift/AST/ASTTypeIDZone.def

@@ -26,6 +26,7 @@ SWIFT_TYPEID(Type)
 SWIFT_TYPEID(TypePair)
 SWIFT_TYPEID_NAMED(CustomAttr *, CustomAttr)
 SWIFT_TYPEID_NAMED(Decl *, Decl)
+SWIFT_TYPEID_NAMED(EnumDecl *, EnumDecl)
 SWIFT_TYPEID_NAMED(GenericParamList *, GenericParamList)
 SWIFT_TYPEID_NAMED(GenericTypeParamType *, GenericTypeParamType)
 SWIFT_TYPEID_NAMED(InfixOperatorDecl *, InfixOperatorDecl)

include/swift/AST/ASTTypeIDs.h

@@ -25,6 +25,7 @@ class AbstractFunctionDecl;
 class BraceStmt;
 class CustomAttr;
 class Decl;
+class EnumDecl;
 class GenericParamList;
 class GenericSignature;
 class GenericTypeParamType;

include/swift/AST/Decl.h

@@ -3519,12 +3519,37 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
 class EnumDecl final : public NominalTypeDecl {
   SourceLoc EnumLoc;
 
+  enum SemanticInfoFlags : uint8_t {
+    // Is the raw type valid?
+    HasComputedRawType         = 1 << 0,
+    // Is the complete set of (auto-incremented) raw values available?
+    HasFixedRawValues          = 1 << 1,
+    // Is the complete set of raw values type checked?
+    HasFixedRawValuesAndTypes  = 1 << 2,
+  };
+  
   struct {
     /// The raw type and a bit to indicate whether the
     /// raw was computed yet or not.
-    llvm::PointerIntPair<Type, 1, bool> RawType;
+    llvm::PointerIntPair<Type, 3, OptionSet<SemanticInfoFlags>> RawTypeAndFlags;
+    
+    bool hasRawType() const {
+      return RawTypeAndFlags.getInt().contains(HasComputedRawType);
+    }
+    void cacheRawType(Type ty) {
+      auto flags = RawTypeAndFlags.getInt() | HasComputedRawType;
+      RawTypeAndFlags.setPointerAndInt(ty, flags);
+    }
+    
+    bool hasFixedRawValues() const {
+      return RawTypeAndFlags.getInt().contains(HasFixedRawValues);
+    }
+    bool hasCheckedRawValues() const {
+      return RawTypeAndFlags.getInt().contains(HasFixedRawValuesAndTypes);
+    }
   } LazySemanticInfo;
 
+  friend class EnumRawValuesRequest;
   friend class EnumRawTypeRequest;
   friend class TypeChecker;
 
@@ -3583,6 +3608,9 @@ class EnumDecl final : public NominalTypeDecl {
     Bits.EnumDecl.Circularity = static_cast<unsigned>(circularity);
   }
 
+  /// Record that this enum has had all of its raw values computed.
+  void setHasFixedRawValues();
+  
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {
     return D->getKind() == DeclKind::Enum;
@@ -3612,9 +3640,11 @@ class EnumDecl final : public NominalTypeDecl {
 
   /// Set the raw type of the enum from its inheritance clause.
   void setRawType(Type rawType) {
-    LazySemanticInfo.RawType.setPointerAndInt(rawType, true);
+    auto flags = LazySemanticInfo.RawTypeAndFlags.getInt();
+    LazySemanticInfo.RawTypeAndFlags.setPointerAndInt(
+        rawType, flags | HasComputedRawType);
   }
-  
+
   /// True if none of the enum cases have associated values.
   ///
   /// Note that this is true for enums with absolutely no cases.
@@ -6354,6 +6384,8 @@ class EnumCaseDecl final : public Decl,
 /// parent EnumDecl, although syntactically they are subordinate to the
 /// EnumCaseDecl.
 class EnumElementDecl : public DeclContext, public ValueDecl {
+  friend class EnumRawValuesRequest;
+  
   /// This is the type specified with the enum element, for
   /// example 'Int' in 'case Y(Int)'.  This is null if there is no type
   /// associated with this element, as in 'case Z' or in all elements of enum
@@ -6394,9 +6426,20 @@ class EnumElementDecl : public DeclContext, public ValueDecl {
 
   ParameterList *getParameterList() const { return Params; }
 
-  bool hasRawValueExpr() const { return RawValueExpr; }
-  LiteralExpr *getRawValueExpr() const { return RawValueExpr; }
-  void setRawValueExpr(LiteralExpr *e) { RawValueExpr = e; }
+  /// Retrieves a fully typechecked raw value expression associated
+  /// with this enum element, if it exists.
+  LiteralExpr *getRawValueExpr() const;
+  
+  /// Retrieves a "structurally" checked raw value expression associated
+  /// with this enum element, if it exists.
+  ///
+  /// The structural raw value may or may not have a type set, but it is
+  /// guaranteed to be suitable for retrieving any non-semantic information
+  /// like digit text for an integral raw value or user text for a string raw value.
+  LiteralExpr *getStructuralRawValueExpr() const;
+  
+  /// Reset the raw value expression.
+  void setRawValueExpr(LiteralExpr *e);
 
   /// Return the containing EnumDecl.
   EnumDecl *getParentEnum() const {
@@ -6419,6 +6462,10 @@ class EnumElementDecl : public DeclContext, public ValueDecl {
   bool isIndirect() const {
     return getAttrs().hasAttribute<IndirectAttr>();
   }
+  
+  /// Do not call this!
+  /// It exists to let the AST walkers get the raw value without forcing a request.
+  LiteralExpr *getRawValueUnchecked() const { return RawValueExpr; }
 
   static bool classof(const Decl *D) {
     return D->getKind() == DeclKind::EnumElement;

include/swift/AST/TypeCheckRequests.h

@@ -1230,6 +1230,31 @@ class OperatorPrecedenceGroupRequest
   bool isCached() const { return true; }
 };
 
+class EnumRawValuesRequest :
+    public SimpleRequest<EnumRawValuesRequest,
+                         bool (EnumDecl *, TypeResolutionStage),
+                         CacheKind::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+  
+private:
+  friend SimpleRequest;
+  
+  // Evaluation.
+  llvm::Expected<bool>
+  evaluate(Evaluator &evaluator, EnumDecl *ED, TypeResolutionStage stage) const;
+  
+public:
+  // Cycle handling.
+  void diagnoseCycle(DiagnosticEngine &diags) const;
+  void noteCycleStep(DiagnosticEngine &diags) const;
+                           
+  // Separate caching.
+  bool isCached() const;
+  Optional<bool> getCachedResult() const;
+  void cacheResult(bool value) const;
+};
+
 // Allow AnyValue to compare two Type values, even though Type doesn't
 // support ==.
 template<>

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -36,6 +36,9 @@ SWIFT_REQUEST(TypeChecker, DefaultTypeRequest,
               NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, EmittedMembersRequest, DeclRange(ClassDecl *),
               SeparatelyCached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, EnumRawValuesRequest,
+              bool (EnumDecl *, TypeResolutionStage), SeparatelyCached,
+              NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, EnumRawTypeRequest,
               Type(EnumDecl *, TypeResolutionStage), SeparatelyCached,
               NoLocationInfo)

lib/AST/ASTPrinter.cpp

@@ -2893,7 +2893,7 @@ void PrintAST::printEnumElement(EnumElementDecl *elt) {
     break;
   }
 
-  auto *raw = elt->getRawValueExpr();
+  auto *raw = elt->getStructuralRawValueExpr();
   if (!raw || raw->isImplicit())
     return;
 

lib/AST/ASTScopeCreation.cpp

@@ -1346,8 +1346,12 @@ void EnumElementScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
   if (auto *pl = decl->getParameterList())
     scopeCreator.constructExpandAndInsertUncheckable<ParameterListScope>(
         this, pl, nullptr);
-  // might contain a closure
-  scopeCreator.addToScopeTree(decl->getRawValueExpr(), this);
+  // The invariant that the raw value expression can never introduce a new scope
+  // is checked in Parse.  However, this guarantee is not future-proof.  Compute
+  // and add the raw value expression anyways just to be defensive.
+  //
+  // FIXME: Re-enable this.  It currently crashes for malformed enum cases.
+  // scopeCreator.addToScopeTree(decl->getStructuralRawValueExpr(), this);
 }
 
 void AbstractFunctionBodyScope::expandAScopeThatDoesNotCreateANewInsertionPoint(

lib/AST/ASTWalker.cpp

@@ -403,7 +403,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
       visit(PL);
     }
 
-    if (auto *rawLiteralExpr = ED->getRawValueExpr()) {
+    if (auto *rawLiteralExpr = ED->getRawValueUnchecked()) {
       Expr *newRawExpr = doIt(rawLiteralExpr);
       if (auto newRawLiteralExpr = dyn_cast<LiteralExpr>(newRawExpr))
         ED->setRawValueExpr(newRawLiteralExpr);

lib/AST/Decl.cpp

@@ -4292,6 +4292,12 @@ bool EnumDecl::isEffectivelyExhaustive(ModuleDecl *M,
             "these should match up");
   return !isResilient(M, expansion);
 }
+      
+void EnumDecl::setHasFixedRawValues() {
+  auto flags = LazySemanticInfo.RawTypeAndFlags.getInt() |
+      EnumDecl::HasFixedRawValues;
+  LazySemanticInfo.RawTypeAndFlags.setInt(flags);
+}
 
 ProtocolDecl::ProtocolDecl(DeclContext *DC, SourceLoc ProtocolLoc,
                            SourceLoc NameLoc, Identifier Name,
@@ -7197,6 +7203,32 @@ EnumCaseDecl *EnumElementDecl::getParentCase() const {
 
   llvm_unreachable("enum element not in case of parent enum");
 }
+      
+LiteralExpr *EnumElementDecl::getRawValueExpr() const {
+  // The return value of this request is irrelevant - it exists as
+  // a cache-warmer.
+  (void)evaluateOrDefault(
+      getASTContext().evaluator,
+      EnumRawValuesRequest{getParentEnum(), TypeResolutionStage::Interface},
+      true);
+  return RawValueExpr;
+}
+
+LiteralExpr *EnumElementDecl::getStructuralRawValueExpr() const {
+  // The return value of this request is irrelevant - it exists as
+  // a cache-warmer.
+  (void)evaluateOrDefault(
+      getASTContext().evaluator,
+      EnumRawValuesRequest{getParentEnum(), TypeResolutionStage::Structural},
+      true);
+  return RawValueExpr;
+}
+
+void EnumElementDecl::setRawValueExpr(LiteralExpr *e) {
+  assert((!RawValueExpr || e == RawValueExpr || e->getType()) &&
+         "Illegal mutation of raw value expr");
+  RawValueExpr = e;
+}
 
 SourceRange ConstructorDecl::getSourceRange() const {
   if (isImplicit())

lib/AST/TypeCheckRequests.cpp

@@ -167,15 +167,15 @@ bool EnumRawTypeRequest::isCached() const {
 
 Optional<Type> EnumRawTypeRequest::getCachedResult() const {
   auto enumDecl = std::get<0>(getStorage());
-  if (enumDecl->LazySemanticInfo.RawType.getInt())
-    return enumDecl->LazySemanticInfo.RawType.getPointer();
+  if (enumDecl->LazySemanticInfo.hasRawType())
+    return enumDecl->LazySemanticInfo.RawTypeAndFlags.getPointer();
 
   return None;
 }
 
 void EnumRawTypeRequest::cacheResult(Type value) const {
   auto enumDecl = std::get<0>(getStorage());
-  enumDecl->LazySemanticInfo.RawType.setPointerAndInt(value, true);
+  enumDecl->LazySemanticInfo.cacheRawType(value);
 }
 
 //----------------------------------------------------------------------------//
@@ -839,7 +839,7 @@ void InferredGenericSignatureRequest::noteCycleStep(DiagnosticEngine &d) const {
 }
 
 //----------------------------------------------------------------------------//
-// IsImplicitlyUnwrappedOptionalRequest computation.
+// UnderlyingTypeRequest computation.
 //----------------------------------------------------------------------------//
 
 Optional<Type>
@@ -854,3 +854,36 @@ void UnderlyingTypeRequest::cacheResult(Type value) const {
   auto *typeAlias = std::get<0>(getStorage());
   typeAlias->UnderlyingTy.setType(value);
 }
+
+//----------------------------------------------------------------------------//
+// EnumRawValuesRequest computation.
+//----------------------------------------------------------------------------//
+
+bool EnumRawValuesRequest::isCached() const {
+  return std::get<1>(getStorage()) == TypeResolutionStage::Interface;
+}
+
+Optional<bool> EnumRawValuesRequest::getCachedResult() const {
+  auto *ED = std::get<0>(getStorage());
+  if (ED->LazySemanticInfo.hasCheckedRawValues())
+    return true;
+  return None;
+}
+
+void EnumRawValuesRequest::cacheResult(bool) const {
+  auto *ED = std::get<0>(getStorage());
+  auto flags = ED->LazySemanticInfo.RawTypeAndFlags.getInt() |
+      EnumDecl::HasFixedRawValues |
+      EnumDecl::HasFixedRawValuesAndTypes;
+  ED->LazySemanticInfo.RawTypeAndFlags.setInt(flags);
+}
+
+void EnumRawValuesRequest::diagnoseCycle(DiagnosticEngine &diags) const {
+  // This request computes the raw type, and so participates in cycles involving
+  // it. For now, the raw type provides a rich enough circularity diagnostic
+  // that we can silence ourselves.
+}
+
+void EnumRawValuesRequest::noteCycleStep(DiagnosticEngine &diags) const {
+
+}

lib/IDE/SyntaxModel.cpp

@@ -916,7 +916,7 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
                                          EnumElemD->getName().getLength());
         }
 
-        if (auto *E = EnumElemD->getRawValueExpr()) {
+        if (auto *E = EnumElemD->getRawValueUnchecked()) {
           SourceRange ElemRange = E->getSourceRange();
           SN.Elements.emplace_back(SyntaxStructureElementKind::InitExpr,
                                  charSourceRangeFromSourceRange(SM, ElemRange));

lib/IRGen/GenEnum.cpp

@@ -1128,7 +1128,7 @@ namespace {
     int64_t getDiscriminatorIndex(EnumElementDecl *target) const override {
       // The elements are assigned discriminators ABI-compatible with their
       // raw values from C.
-      assert(target->hasRawValueExpr()
+      assert(target->getRawValueExpr()
              && "c-compatible enum elt has no raw value?!");
       auto intExpr = cast<IntegerLiteralExpr>(target->getRawValueExpr());
       auto intType = getDiscriminatorType();

lib/PrintAsObjC/DeclAndTypePrinter.cpp

@@ -393,7 +393,8 @@ class DeclAndTypePrinter::Implementation
         os << " SWIFT_COMPILE_NAME(\"" << Elt->getName() << "\")";
       }
 
-      if (auto ILE = cast_or_null<IntegerLiteralExpr>(Elt->getRawValueExpr())) {
+      auto *RVE = Elt->getStructuralRawValueExpr();
+      if (auto ILE = cast_or_null<IntegerLiteralExpr>(RVE)) {
         os << " = ";
         if (ILE->isNegative())
           os << "-";