Merge pull request #27777 from CodaFi/a-circular-hierarchy-of-needs

CodaFi · web-flow · commit 414573744e2e · 2019-10-18T12:49:05.000-07:00
Delete DeclValidationRAII
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -282,17 +282,10 @@ bool conflicting(ASTContext &ctx,
 
 /// Decl - Base class for all declarations in Swift.
 class alignas(1 << DeclAlignInBits) Decl {
-public:
-  enum class ValidationState {
-    Unchecked,
-    Checking,
-    Checked,
-  };
-
 protected:
   union { uint64_t OpaqueBits;
 
-  SWIFT_INLINE_BITFIELD_BASE(Decl, bitmax(NumDeclKindBits,8)+1+1+1+1+2+1,
+  SWIFT_INLINE_BITFIELD_BASE(Decl, bitmax(NumDeclKindBits,8)+1+1+1+1+1,
     Kind : bitmax(NumDeclKindBits,8),
 
     /// Whether this declaration is invalid.
@@ -307,9 +300,6 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// Use getClangNode() to retrieve the corresponding Clang AST.
     FromClang : 1,
 
-    /// The validation state of this declaration.
-    ValidationState : 2,
-
     /// Whether this declaration was added to the surrounding
     /// DeclContext of an active #if config clause.
     EscapedFromIfConfig : 1
@@ -689,7 +679,6 @@ class alignas(1 << DeclAlignInBits) Decl {
     Bits.Decl.Invalid = false;
     Bits.Decl.Implicit = false;
     Bits.Decl.FromClang = false;
-    Bits.Decl.ValidationState = unsigned(ValidationState::Unchecked);
     Bits.Decl.EscapedFromIfConfig = false;
   }
 
@@ -830,37 +819,7 @@ class alignas(1 << DeclAlignInBits) Decl {
   /// Mark this declaration as implicit.
   void setImplicit(bool implicit = true) { Bits.Decl.Implicit = implicit; }
 
-  /// Get the validation state.
-  ValidationState getValidationState() const {
-    return ValidationState(Bits.Decl.ValidationState);
-  }
-
-private:
-  friend class DeclValidationRAII;
-
-  /// Set the validation state.
-  void setValidationState(ValidationState VS) {
-    assert(VS > getValidationState() && "Validation is unidirectional");
-    Bits.Decl.ValidationState = unsigned(VS);
-  }
-
 public:
-  /// Whether the declaration is in the middle of validation or not.
-  bool isBeingValidated() const {
-    switch (getValidationState()) {
-    case ValidationState::Unchecked:
-    case ValidationState::Checked:
-      return false;
-    case ValidationState::Checking:
-      return true;
-    }
-    llvm_unreachable("Unknown ValidationState");
-  }
-
-  bool hasValidationStarted() const {
-    return getValidationState() > ValidationState::Unchecked;
-  }
-
   bool escapedFromIfConfig() const {
     return Bits.Decl.EscapedFromIfConfig;
   }
@@ -982,25 +941,6 @@ class alignas(1 << DeclAlignInBits) Decl {
   }
 };
 
-/// Use RAII to track Decl validation progress and non-reentrancy.
-class DeclValidationRAII {
-  Decl *D;
-
-public:
-  DeclValidationRAII(const DeclValidationRAII &) = delete;
-  DeclValidationRAII(DeclValidationRAII &&) = delete;
-  void operator =(const DeclValidationRAII &) = delete;
-  void operator =(DeclValidationRAII &&) = delete;
-
-  DeclValidationRAII(Decl *decl) : D(decl) {
-    D->setValidationState(Decl::ValidationState::Checking);
-  }
-
-  ~DeclValidationRAII() {
-    D->setValidationState(Decl::ValidationState::Checked);
-  }
-};
-
 /// Allocates memory for a Decl with the given \p baseSize. If necessary,
 /// it includes additional space immediately preceding the Decl for a ClangNode.
 /// \note \p baseSize does not need to include space for a ClangNode if
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -883,6 +883,8 @@ NOTE(found_this_precedence_group,none,
 ERROR(unknown_precedence_group,none,
       "unknown precedence group %0", (Identifier))
 ERROR(precedence_group_cycle,none,
+      "cycle in '%select{lowerThan|higherThan}0' relation", (bool))
+ERROR(higher_than_precedence_group_cycle,none,
       "cycle in higherThan relation: %0", (StringRef))
 ERROR(precedence_group_lower_within_module,none,
       "precedence group cannot be given lower precedence than group in same"
@@ -892,6 +894,8 @@ ERROR(precedence_group_redeclared,none,
       "precedence group redeclared", ())
 NOTE(previous_precedence_group_decl,none,
      "previous precedence group declaration here", ())
+NOTE(circular_reference_through_precedence_group, none,
+     "through reference to precedence group %0 here", (Identifier))
 
 //------------------------------------------------------------------------------
 // MARK: Expression Type Checking Errors
diff --git a/include/swift/AST/NameLookupRequests.h b/include/swift/AST/NameLookupRequests.h
@@ -278,56 +278,6 @@ class GenericParamListRequest :
   void cacheResult(GenericParamList *value) const;
 };
 
-struct PrecedenceGroupDescriptor {
-  DeclContext *dc;
-  Identifier ident;
-  SourceLoc nameLoc;
-
-  SourceLoc getLoc() const;
-
-  friend llvm::hash_code hash_value(const PrecedenceGroupDescriptor &owner) {
-    return hash_combine(llvm::hash_value(owner.dc),
-                        llvm::hash_value(owner.ident.getAsOpaquePointer()),
-                        llvm::hash_value(owner.nameLoc.getOpaquePointerValue()));
-  }
-
-  friend bool operator==(const PrecedenceGroupDescriptor &lhs,
-                         const PrecedenceGroupDescriptor &rhs) {
-    return lhs.dc == rhs.dc &&
-           lhs.ident == rhs.ident &&
-           lhs.nameLoc == rhs.nameLoc;
-  }
-
-  friend bool operator!=(const PrecedenceGroupDescriptor &lhs,
-                         const PrecedenceGroupDescriptor &rhs) {
-    return !(lhs == rhs);
-  }
-};
-
-void simple_display(llvm::raw_ostream &out, const PrecedenceGroupDescriptor &d);
-
-class LookupPrecedenceGroupRequest
-    : public SimpleRequest<LookupPrecedenceGroupRequest,
-                           PrecedenceGroupDecl *(PrecedenceGroupDescriptor),
-                           CacheKind::Cached> {
-public:
-  using SimpleRequest::SimpleRequest;
-
-private:
-  friend SimpleRequest;
-
-  // Evaluation.
-  llvm::Expected<PrecedenceGroupDecl *>
-  evaluate(Evaluator &evaluator, PrecedenceGroupDescriptor descriptor) const;
-
-public:
-  // Source location
-  SourceLoc getNearestLoc() const;
-                               
-  // Separate caching.
-  bool isCached() const { return true; }
-};
-
 /// Expand the given ASTScope. Requestified to detect recursion.
 class ExpandASTScopeRequest
     : public SimpleRequest<ExpandASTScopeRequest,
diff --git a/include/swift/AST/NameLookupTypeIDZone.def b/include/swift/AST/NameLookupTypeIDZone.def
@@ -34,9 +34,6 @@ SWIFT_REQUEST(NameLookup, InheritedDeclsReferencedRequest,
               DirectlyReferencedTypeDecls(
                   llvm::PointerUnion<TypeDecl *, ExtensionDecl *>, unsigned),
               Uncached, HasNearestLocation)
-SWIFT_REQUEST(NameLookup, LookupPrecedenceGroupRequest,
-              PrecedenceGroupDecl *(DeclContext *, Identifier, SourceLoc),
-              Cached, NoLocationInfo)
 SWIFT_REQUEST(NameLookup, SelfBoundsFromWhereClauseRequest,
               SelfBounds(llvm::PointerUnion<TypeDecl *, ExtensionDecl *>),
               Uncached, NoLocationInfo)
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -1448,6 +1448,66 @@ class InterfaceTypeRequest :
   void cacheResult(Type value) const;
 };
 
+struct PrecedenceGroupDescriptor {
+  enum PathDirection : bool {
+    LowerThan = false,
+    HigherThan = true,
+  };
+  DeclContext *dc;
+  Identifier ident;
+  SourceLoc nameLoc;
+  // Exists for diagnostics. Does not contribute to the descriptor otherwise.
+  Optional<PathDirection> pathDirection;
+
+  SourceLoc getLoc() const;
+
+  friend llvm::hash_code hash_value(const PrecedenceGroupDescriptor &owner) {
+    return llvm::hash_combine(owner.dc,
+                              owner.ident.getAsOpaquePointer(),
+                              owner.nameLoc.getOpaquePointerValue());
+  }
+
+  friend bool operator==(const PrecedenceGroupDescriptor &lhs,
+                         const PrecedenceGroupDescriptor &rhs) {
+    return lhs.dc == rhs.dc &&
+           lhs.ident == rhs.ident &&
+           lhs.nameLoc == rhs.nameLoc;
+  }
+
+  friend bool operator!=(const PrecedenceGroupDescriptor &lhs,
+                         const PrecedenceGroupDescriptor &rhs) {
+    return !(lhs == rhs);
+  }
+};
+
+void simple_display(llvm::raw_ostream &out, const PrecedenceGroupDescriptor &d);
+
+class LookupPrecedenceGroupRequest
+    : public SimpleRequest<LookupPrecedenceGroupRequest,
+                           PrecedenceGroupDecl *(PrecedenceGroupDescriptor),
+                           CacheKind::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::Expected<PrecedenceGroupDecl *>
+  evaluate(Evaluator &evaluator, PrecedenceGroupDescriptor descriptor) const;
+
+public:
+  // Cycle handling.
+  void diagnoseCycle(DiagnosticEngine &diags) const;
+  void noteCycleStep(DiagnosticEngine &diags) const;
+
+  // Source location
+  SourceLoc getNearestLoc() const;
+
+  // Separate caching.
+  bool isCached() const { return true; }
+};
+
 // Allow AnyValue to compare two Type values, even though Type doesn't
 // support ==.
 template<>
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -85,6 +85,9 @@ SWIFT_REQUEST(TypeChecker, IsSetterMutatingRequest, bool(AbstractStorageDecl *),
               SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, LazyStoragePropertyRequest, VarDecl *(VarDecl *),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, LookupPrecedenceGroupRequest,
+              PrecedenceGroupDecl *(DeclContext *, Identifier, SourceLoc),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, MangleLocalTypeDeclRequest,
               std::string(const TypeDecl *), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, NamingPatternRequest,
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -2783,8 +2783,13 @@ bool ValueDecl::hasInterfaceType() const {
   return !TypeAndAccess.getPointer().isNull();
 }
 
+static bool isComputingInterfaceType(const ValueDecl *VD) {
+  return VD->getASTContext().evaluator.hasActiveRequest(
+            InterfaceTypeRequest{const_cast<ValueDecl *>(VD)});
+}
+
 bool ValueDecl::isRecursiveValidation() const {
-  if (hasValidationStarted() && !hasInterfaceType())
+  if (isComputingInterfaceType(this) && !hasInterfaceType())
     return true;
 
   if (auto *vd = dyn_cast<VarDecl>(this))
@@ -7404,21 +7409,6 @@ PrecedenceGroupDecl::PrecedenceGroupDecl(DeclContext *dc,
          lowerThan.size() * sizeof(Relation));
 }
 
-llvm::Expected<PrecedenceGroupDecl *> LookupPrecedenceGroupRequest::evaluate(
-    Evaluator &eval, PrecedenceGroupDescriptor descriptor) const {
-  auto *dc = descriptor.dc;
-  PrecedenceGroupDecl *group = nullptr;
-  if (auto sf = dc->getParentSourceFile()) {
-    bool cascading = dc->isCascadingContextForLookup(false);
-    group = sf->lookupPrecedenceGroup(descriptor.ident, cascading,
-                                      descriptor.nameLoc);
-  } else {
-    group = dc->getParentModule()->lookupPrecedenceGroup(descriptor.ident,
-                                                         descriptor.nameLoc);
-  }
-  return group;
-}
-
 PrecedenceGroupDecl *InfixOperatorDecl::getPrecedenceGroup() const {
   return evaluateOrDefault(
       getASTContext().evaluator,
diff --git a/lib/AST/NameLookupRequests.cpp b/lib/AST/NameLookupRequests.cpp
@@ -128,26 +128,6 @@ void GenericParamListRequest::cacheResult(GenericParamList *params) const {
   context->GenericParamsAndBit.setPointerAndInt(params, true);
 }
 
-
-//----------------------------------------------------------------------------//
-// LookupPrecedenceGroupRequest computation.
-//----------------------------------------------------------------------------//
-
-SourceLoc LookupPrecedenceGroupRequest::getNearestLoc() const {
-  auto &desc = std::get<0>(getStorage());
-  return desc.getLoc();
-}
-
-SourceLoc PrecedenceGroupDescriptor::getLoc() const {
-  return nameLoc;
-}
-
-void swift::simple_display(llvm::raw_ostream &out,
-                           const PrecedenceGroupDescriptor &desc) {
-  out << "precedence group " << desc.ident << " at ";
-  desc.nameLoc.print(out, desc.dc->getASTContext().SourceMgr);
-}
-
 // Define request evaluation functions for each of the name lookup requests.
 static AbstractRequestFunction *nameLookupRequestFunctions[] = {
 #define SWIFT_REQUEST(Zone, Name, Sig, Caching, LocOptions)                    \
diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp
@@ -1026,3 +1026,37 @@ void InterfaceTypeRequest::cacheResult(Type type) const {
   }
   decl->TypeAndAccess.setPointer(type);
 }
+
+//----------------------------------------------------------------------------//
+// LookupPrecedenceGroupRequest computation.
+//----------------------------------------------------------------------------//
+
+SourceLoc LookupPrecedenceGroupRequest::getNearestLoc() const {
+  auto &desc = std::get<0>(getStorage());
+  return desc.getLoc();
+}
+
+void LookupPrecedenceGroupRequest::diagnoseCycle(DiagnosticEngine &diags) const {
+  auto &desc = std::get<0>(getStorage());
+  if (auto pathDir = desc.pathDirection) {
+    diags.diagnose(desc.nameLoc, diag::precedence_group_cycle, (bool)*pathDir);
+  } else {
+    diags.diagnose(desc.nameLoc, diag::circular_reference);
+  }
+}
+
+void LookupPrecedenceGroupRequest::noteCycleStep(DiagnosticEngine &diag) const {
+  auto &desc = std::get<0>(getStorage());
+  diag.diagnose(desc.nameLoc,
+                 diag::circular_reference_through_precedence_group, desc.ident);
+}
+
+SourceLoc PrecedenceGroupDescriptor::getLoc() const {
+  return nameLoc;
+}
+
+void swift::simple_display(llvm::raw_ostream &out,
+                           const PrecedenceGroupDescriptor &desc) {
+  out << "precedence group " << desc.ident << " at ";
+  desc.nameLoc.print(out, desc.dc->getASTContext().SourceMgr);
+}
diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp
diff --git a/test/decl/precedencegroup/circularity.swift b/test/decl/precedencegroup/circularity.swift
