---

yaml
---
r: 323543
b: refs/heads/tensorflow-next
c: 7a97cd8
h: refs/heads/master
i:
  323541: 492bff3
  323539: 7fb4406
  323535: abb8172

Author: swift-ci

[refs]

@@ -1461,4 +1461,4 @@ refs/heads/master-rebranch: 86e95c23aa0d37f24ec138b7853146c1cead2e40
 refs/heads/rdar-53901732: 9bd06af3284e18a109cdbf9aa59d833b24eeca7b
 refs/heads/revert-26776-subst-always-returns-a-type: 1b8e18fdd391903a348970a4c848995d4cdd789c
 refs/heads/tensorflow-merge: 8b854f62f80d4476cb383d43c4aac2001dde3cec
-refs/heads/tensorflow-next: ab7dca2933ed265fff7347be68e9b9fdeade85a7
+refs/heads/tensorflow-next: 7a97cd8314c1b6d5de114b5c2a75c91c8274a9ad

branches/tensorflow-next/docs/RequestEvaluator.md

@@ -58,9 +58,7 @@ The request-evaluator is relatively new to the Swift compiler, having been intro
 
 * The evaluator uses a `DenseMap<AnyRequest, AnyValue>` as its cache: we can almost certainly do better with per-request-kind caches that don't depend on so much type erasure.
 * The stack of active requests uses a `SetVector<AnyRequest>`: we can almost certainly do better with some kind of heterogeneous on-stack representation that only realizes `AnyRequest` instances in the failure cases (e.g., to diagnose a cycle).
-* Add API to the evaluator to add a specific (request, result) pair to the cache, and make it "stick" even if the cache is cleared. This API should be usable from places in the compiler that inject specific known state, such as when the Clang importer synthesizes ASTs.
 * Explore how best to cache data structures in the evaluator. For example, caching `std::vector<T>` or `std::string` implies that we'll make copies of the underlying data structure each time we access the data. Could we automatically intern the data into an allocation arena owned by the evaluator, and vend `ArrayRef<T>` and `StringRef` to clients instead?
-* Sometimes the compiler crashes or otherwise fails due to a cycle detected by the evaluator but (currently) not diagnosed. Add some logic to record cycles encountered during compilation, and if the compiler crashes (or otherwise fails), dump the cycles at the end of execution to help with debugging.
 * Cycle diagnostics are far too complicated and produce very poor results. Consider replacing the current `diagnoseCycle`/`noteCycleStep` scheme with a single method that produces summary information (e.g., a short summary string + source location information) and provides richer diagnostics from that string.
 * The `isCached()` check to determine whether a specific instance of a request is worth caching may be at the wrong level, because one generally has to duplicate effort (or worse, code!) to make the decision in `isCached()`. Consider whether the `evaluator()` function could return something special to say "produce this value without caching" vs. the normal "produce this value with caching".
 * Try to eliminate more boilerplate from subclasses of [`SimpleRequest`](https://github.com/apple/swift/blob/master/include/swift/AST/SimpleRequest.h). We are going to have a *lot* of requests.

branches/tensorflow-next/include/swift/AST/AccessRequests.h

@@ -45,10 +45,6 @@ class AccessLevelRequest :
                                        ValueDecl *decl) const;
 
 public:
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
-
   // Separate caching.
   bool isCached() const { return true; }
   Optional<AccessLevel> getCachedResult() const;
@@ -73,10 +69,6 @@ class SetterAccessLevelRequest :
   evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;
 
 public:
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
-
   // Separate caching.
   bool isCached() const { return true; }
   Optional<AccessLevel> getCachedResult() const;
@@ -99,10 +91,6 @@ class DefaultAndMaxAccessLevelRequest :
   evaluate(Evaluator &evaluator, ExtensionDecl *decl) const;
 
 public:
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
-
   // Separate caching.
   bool isCached() const { return true; }
   Optional<DefaultAndMax> getCachedResult() const;

branches/tensorflow-next/include/swift/AST/Attr.h

@@ -1680,6 +1680,10 @@ class DeclAttributes {
 
 void simple_display(llvm::raw_ostream &out, const DeclAttribute *attr);
 
+inline SourceLoc extractNearestSourceLoc(const DeclAttribute *attr) {
+  return attr->getLocation();
+}
+
 } // end namespace swift
 
 #endif

branches/tensorflow-next/include/swift/AST/Decl.h

@@ -7345,6 +7345,24 @@ void simple_display(llvm::raw_ostream &out, const Decl *decl);
 /// Display ValueDecl subclasses.
 void simple_display(llvm::raw_ostream &out, const ValueDecl *decl);
 
+/// Extract the source location from the given declaration.
+SourceLoc extractNearestSourceLoc(const Decl *decl);
+
+/// Extract the source location from the given declaration.
+inline SourceLoc extractNearestSourceLoc(const ExtensionDecl *ext) {
+  return extractNearestSourceLoc(static_cast<const Decl *>(ext));
+}
+
+/// Extract the source location from the given declaration.
+inline SourceLoc extractNearestSourceLoc(const GenericTypeDecl *type) {
+  return extractNearestSourceLoc(static_cast<const Decl *>(type));
+}
+
+/// Extract the source location from the given declaration.
+inline SourceLoc extractNearestSourceLoc(const AbstractFunctionDecl *func) {
+  return extractNearestSourceLoc(static_cast<const Decl *>(func));
+}
+
 } // end namespace swift
 
 #endif

branches/tensorflow-next/include/swift/AST/DeclContext.h

@@ -787,6 +787,9 @@ void simple_display(llvm::raw_ostream &out, const ParamT *dc) {
     out << "(null)";
 }
 
+/// Extract the source location from the given declaration context.
+SourceLoc extractNearestSourceLoc(const DeclContext *dc);
+
 } // end namespace swift
 
 namespace llvm {

branches/tensorflow-next/include/swift/AST/NameLookupRequests.h

@@ -81,9 +81,8 @@ class InheritedDeclsReferencedRequest :
   // Caching
   bool isCached() const { return true; }
 
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
+  // Source location information.
+  SourceLoc getNearestLoc() const;
 };
 
 /// Request the set of declarations directly referenced by the underlying
@@ -124,10 +123,6 @@ class UnderlyingTypeDeclsReferencedRequest :
 public:
   // Caching
   bool isCached() const { return true; }
-
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
 };
 
 /// Request the superclass declaration for the given class.
@@ -150,10 +145,6 @@ class SuperclassDeclRequest :
   bool isCached() const { return true; }
   Optional<ClassDecl *> getCachedResult() const;
   void cacheResult(ClassDecl *value) const;
-
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
 };
 
 /// Request the nominal declaration extended by a given extension declaration.
@@ -176,10 +167,6 @@ class ExtendedNominalRequest :
   bool isCached() const { return true; }
   Optional<NominalTypeDecl *> getCachedResult() const;
   void cacheResult(NominalTypeDecl *value) const;
-
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
 };
 
 struct SelfBounds {
@@ -192,7 +179,7 @@ struct SelfBounds {
 class SelfBoundsFromWhereClauseRequest :
     public SimpleRequest<SelfBoundsFromWhereClauseRequest,
                          SelfBounds(llvm::PointerUnion<TypeDecl *,
-                                    ExtensionDecl *>),
+                                                       ExtensionDecl *>),
                          CacheKind::Uncached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -203,12 +190,6 @@ class SelfBoundsFromWhereClauseRequest :
   // Evaluation.
   SelfBounds evaluate(Evaluator &evaluator,
                       llvm::PointerUnion<TypeDecl *, ExtensionDecl *>) const;
-
-public:
-  // Cycle handling
-  SelfBounds breakCycle() const { return { }; }
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
 };
 
 
@@ -227,12 +208,6 @@ class TypeDeclsFromWhereClauseRequest :
   // Evaluation.
   DirectlyReferencedTypeDecls evaluate(Evaluator &evaluator,
                                        ExtensionDecl *ext) const;
-
-public:
-  // Cycle handling
-  DirectlyReferencedTypeDecls breakCycle() const { return { }; }
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
 };
 
 /// Request the nominal type declaration to which the given custom attribute
@@ -254,10 +229,6 @@ class CustomAttrNominalRequest :
 public:
   // Caching
   bool isCached() const { return true; }
-
-  // Cycle handling
-  void diagnoseCycle(DiagnosticEngine &diags) const;
-  void noteCycleStep(DiagnosticEngine &diags) const;
 };
 
 /// The zone number for name-lookup requests.

branches/tensorflow-next/include/swift/AST/SimpleRequest.h

@@ -25,6 +25,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLExtras.h"
 #include <tuple>
+#include <type_traits>
 
 namespace swift {
 
@@ -42,6 +43,99 @@ enum class CacheKind {
   SeparatelyCached,
 };
 
+/// -------------------------------------------------------------------------
+/// Extracting the source location "nearest" a request.
+/// -------------------------------------------------------------------------
+
+namespace detail {
+  /// Dummy extract function used to detect when we can call
+  /// extractNearestSourceLoc() safely.
+  inline void extractNearestSourceLoc(...) { }
+
+  /// Metaprogram to determine whether any input is true.
+  constexpr bool anyTrue() { return false; }
+
+  template<typename ...Rest>
+  constexpr bool anyTrue(bool current, Rest... rest) {
+    return current || anyTrue(rest...);
+  }
+}
+
+/// Determine whether we can extract a nearest source location from a value of
+/// the given type.
+template<typename T>
+constexpr bool canExtractNearestSourceLoc() {
+  using detail::extractNearestSourceLoc;
+  return !std::is_void<decltype(extractNearestSourceLoc(*(T*)nullptr))>::value;
+}
+
+/// Extract source locations when possible, or return an invalid source
+/// location if not possible.
+template<typename T,
+         typename = typename std::enable_if<
+             canExtractNearestSourceLoc<T>()>::type>
+SourceLoc maybeExtractNearestSourceLoc(const T& value) {
+  return extractNearestSourceLoc(value);
+}
+
+template<typename T,
+         typename = void,
+         typename = typename std::enable_if<
+             !canExtractNearestSourceLoc<T>()>::type>
+SourceLoc maybeExtractNearestSourceLoc(const T& value) {
+  return SourceLoc();
+}
+
+/// Extract the nearest source location from a pointer union.
+template<typename T, typename U,
+          typename = typename std::enable_if<
+            canExtractNearestSourceLoc<T>() &&
+            canExtractNearestSourceLoc<U>()>::type>
+SourceLoc extractNearestSourceLoc(const llvm::PointerUnion<T, U> &value) {
+  if (auto first = value.template dyn_cast<T>()) {
+    return extractNearestSourceLoc(first);
+  }
+  if (auto second = value.template dyn_cast<U>()) {
+    return extractNearestSourceLoc(second);
+  }
+  return SourceLoc();
+}
+
+namespace detail {
+  /// Basis case for extracting the nearest source location from a tuple.
+  template<unsigned Index, typename ...Types,
+           typename = void,
+           typename = typename std::enable_if<
+               (Index >= sizeof...(Types))>::type>
+  SourceLoc extractNearestSourceLocTuple(const std::tuple<Types...> &) {
+    return SourceLoc();
+  }
+
+  /// Extract the first, nearest source location from a tuple.
+  template<unsigned Index, typename ...Types,
+           typename = typename std::enable_if<Index < sizeof...(Types)>::type>
+  SourceLoc extractNearestSourceLocTuple(const std::tuple<Types...> &value) {
+    SourceLoc loc = maybeExtractNearestSourceLoc(std::get<Index>(value));
+    if (loc.isValid())
+      return loc;
+
+    return extractNearestSourceLocTuple<Index + 1>(value);
+  }
+}
+
+/// Extract the first, nearest source location from a tuple.
+template<typename First, typename ...Rest,
+         typename = typename std::enable_if<
+             detail::anyTrue(canExtractNearestSourceLoc<First>(),
+                             canExtractNearestSourceLoc<Rest>()...)>::type>
+SourceLoc extractNearestSourceLoc(const std::tuple<First, Rest...> &value) {
+  return detail::extractNearestSourceLocTuple<0>(value);
+}
+
+/// -------------------------------------------------------------------------
+/// Simple Requests
+/// -------------------------------------------------------------------------
+
 /// CRTP base class that describes a request operation that takes values
 /// with the given input types (\c Inputs...) and produces an output of
 /// the given type.
@@ -66,14 +160,14 @@ enum class CacheKind {
 ///   void noteCycleStep(DiagnosticEngine &diags) const;
 /// \endcode
 ///
-/// Or the \c Derived class can provide a "diagnostic location" operation and
-/// diagnostic values for the main cycle diagnostic and a "note" describing a
-/// step within the chain of diagnostics:
+/// Or the implementation will use the default "circular reference" diagnostics
+/// based on the "nearest" source location, which can be provided explicitly by
+/// implementing the following in the subclass:
 /// \code
-///   T getCycleDiagnosticLoc(Inputs...) const;
-///   static constexpr Diag<Inputs...> cycleDiagnostic = ...;
-///   static constexpr Diag<Inputs...> cycleStepDiagnostic = ...;
+///   SourceLoc getNearestLoc() const;
 /// \endcode
+/// If not provided, a default implementation for \c getNearestLoc() will pick
+/// the source location from the first input that provides one.
 ///
 /// Value caching is determined by the \c Caching parameter. When
 /// \c Caching == CacheKind::SeparatelyCached, the \c Derived class is
@@ -106,14 +200,6 @@ class SimpleRequest<Derived, Output(Inputs...), Caching> {
     return asDerived().evaluate(evaluator, std::get<Indices>(storage)...);
   }
 
-  template<size_t ...Indices>
-  void diagnoseImpl(DiagnosticEngine &diags, Diag<Inputs...> diag,
-                    llvm::index_sequence<Indices...>) const {
-    diags.diagnose(
-      asDerived().getCycleDiagnosticLoc(std::get<Indices>(storage)...),
-      diag, std::get<Indices>(storage)...);
-  }
-
 protected:
   /// Retrieve the storage value directly.
   const std::tuple<Inputs...> &getStorage() const { return storage; }
@@ -134,14 +220,18 @@ class SimpleRequest<Derived, Output(Inputs...), Caching> {
                                llvm::index_sequence_for<Inputs...>());
   }
 
+  /// Retrieve the nearest source location to which this request applies.
+  SourceLoc getNearestLoc() const {
+    return extractNearestSourceLoc(storage);
+  }
+
   void diagnoseCycle(DiagnosticEngine &diags) const {
-    diagnoseImpl(diags, Derived::cycleDiagnostic,
-                 llvm::index_sequence_for<Inputs...>());
+    diags.diagnose(asDerived().getNearestLoc(), diag::circular_reference);
   }
 
   void noteCycleStep(DiagnosticEngine &diags) const {
-    diagnoseImpl(diags, Derived::cycleStepDiagnostic,
-                 llvm::index_sequence_for<Inputs...>());
+    diags.diagnose(asDerived().getNearestLoc(),
+                   diag::circular_reference_through);
   }
 
   friend bool operator==(const SimpleRequest &lhs, const SimpleRequest &rhs) {