AST: Always diagnose request evaluator cycles

Author: slavapestov

include/swift/AST/Evaluator.h

@@ -20,7 +20,6 @@
 
 #include "swift/AST/AnyRequest.h"
 #include "swift/Basic/AnyValue.h"
-#include "swift/Basic/CycleDiagnosticKind.h"
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/Statistic.h"
 #include "llvm/ADT/DenseMap.h"
@@ -184,8 +183,8 @@ class Evaluator {
   /// diagnostics will be emitted.
   DiagnosticEngine &diags;
 
-  /// Whether to diagnose cycles or ignore them completely.
-  CycleDiagnosticKind shouldDiagnoseCycles;
+  /// Whether to dump detailed debug info for cycles.
+  bool debugDumpCycles;
 
   /// Used to report statistics about which requests were evaluated, if
   /// non-null.
@@ -237,7 +236,7 @@ class Evaluator {
 public:
   /// Construct a new evaluator that can emit cyclic-dependency
   /// diagnostics through the given diagnostics engine.
-  Evaluator(DiagnosticEngine &diags, CycleDiagnosticKind shouldDiagnoseCycles);
+  Evaluator(DiagnosticEngine &diags, bool debugDumpCycles=false);
 
   /// Emit GraphViz output visualizing the request graph.
   void emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath);

include/swift/Basic/CycleDiagnosticKind.h

@@ -19,7 +19,6 @@
 #define SWIFT_BASIC_LANGOPTIONS_H
 
 #include "swift/Config.h"
-#include "swift/Basic/CycleDiagnosticKind.h"
 #include "swift/Basic/LLVM.h"
 #include "swift/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -185,9 +184,8 @@ namespace swift {
     /// This is for testing purposes.
     std::string DebugForbidTypecheckPrefix;
 
-    /// How to diagnose cycles encountered
-    CycleDiagnosticKind EvaluatorCycleDiagnostics =
-        CycleDiagnosticKind::NoDiagnose;
+    /// Whether to dump debug info for request evaluator cycles.
+    bool DebugDumpCycles = false;
 
     /// The path to which we should emit GraphViz output for the complete
     /// request-evaluator graph.

include/swift/Basic/LangOptions.h

@@ -479,7 +479,7 @@ ASTContext::ASTContext(LangOptions &langOpts, SearchPathOptions &SearchPathOpts,
     SearchPathOpts(SearchPathOpts),
     SourceMgr(SourceMgr),
     Diags(Diags),
-    evaluator(Diags, langOpts.EvaluatorCycleDiagnostics),
+    evaluator(Diags, langOpts.DebugDumpCycles),
     TheBuiltinModule(createBuiltinModule(*this)),
     StdlibModuleName(getIdentifier(STDLIB_NAME)),
     SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),

lib/AST/ASTContext.cpp

@@ -59,9 +59,8 @@ void Evaluator::registerRequestFunctions(
   requestFunctionsByZone.push_back({zoneID, functions});
 }
 
-Evaluator::Evaluator(DiagnosticEngine &diags,
-                     CycleDiagnosticKind shouldDiagnoseCycles)
-  : diags(diags), shouldDiagnoseCycles(shouldDiagnoseCycles) { }
+Evaluator::Evaluator(DiagnosticEngine &diags, bool debugDumpCycles)
+  : diags(diags), debugDumpCycles(debugDumpCycles) { }
 
 void Evaluator::emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath) {
   std::error_code error;
@@ -80,24 +79,16 @@ bool Evaluator::checkDependency(const AnyRequest &request) {
   }
 
   // Diagnose cycle.
-  switch (shouldDiagnoseCycles) {
-  case CycleDiagnosticKind::NoDiagnose:
-    return true;
+  diagnoseCycle(request);
 
-  case CycleDiagnosticKind::DebugDiagnose: {
+  if (debugDumpCycles) {
     llvm::errs() << "===CYCLE DETECTED===\n";
     llvm::DenseSet<AnyRequest> visitedAnywhere;
     llvm::SmallVector<AnyRequest, 4> visitedAlongPath;
     std::string prefixStr;
     printDependencies(activeRequests.front(), llvm::errs(), visitedAnywhere,
                       visitedAlongPath, activeRequests.getArrayRef(),
                       prefixStr, /*lastChild=*/true);
-    return true;
-  }
-
-  case CycleDiagnosticKind::FullDiagnose:
-    diagnoseCycle(request);
-    return true;
   }
 
   return true;

lib/AST/Evaluator.cpp

@@ -360,9 +360,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     Opts.DebugForbidTypecheckPrefix = A->getValue();
   }
 
-  if (Args.getLastArg(OPT_debug_cycles)) {
-    Opts.EvaluatorCycleDiagnostics = CycleDiagnosticKind::DebugDiagnose;
-  }
+  if (Args.getLastArg(OPT_debug_cycles))
+    Opts.DebugDumpCycles = true;
 
   if (const Arg *A = Args.getLastArg(OPT_output_request_graphviz)) {
     Opts.RequestEvaluatorGraphVizPath = A->getValue();

lib/Frontend/CompilerInvocation.cpp

@@ -8,13 +8,13 @@
 // Check that we produced superclass type requests.
 // RUN: %{python} %utils/process-stats-dir.py --evaluate 'SuperclassTypeRequest == 17' %t/stats-dir
 
-class Left
-    : Right.Hand {
+class Left // expected-error {{circular reference}}
+    : Right.Hand { // expected-note {{through reference here}}
   class Hand {}
 }
 
-class Right
-  : Left.Hand {
+class Right // expected-note {{through reference here}}
+  : Left.Hand { // expected-note {{through reference here}}
   class Hand {}
 }
 
@@ -35,14 +35,14 @@ class Outer {
   class Inner : Outer {}
 }
 
-class Outer2
-    : Outer2.Inner {
+class Outer2 // expected-error {{circular reference}}
+    : Outer2.Inner { // expected-note {{through reference here}}
 
   class Inner {}
 }
 
-class Outer3
-    : Outer3.Inner<Int> {
+class Outer3 // expected-error {{circular reference}}
+    : Outer3.Inner<Int> { // expected-note {{through reference here}}
   class Inner<T> {}
 }
 

test/decl/class/circular_inheritance.swift

@@ -211,7 +211,7 @@ TEST(ArithmeticEvaluator, Simple) {
 
   SourceManager sourceMgr;
   DiagnosticEngine diags(sourceMgr);
-  Evaluator evaluator(diags, CycleDiagnosticKind::FullDiagnose);
+  Evaluator evaluator(diags);
   evaluator.registerRequestFunctions(SWIFT_ARITHMETIC_EVALUATOR_ZONE,
                                      arithmeticRequestFunctions);
 
@@ -334,7 +334,7 @@ TEST(ArithmeticEvaluator, Cycle) {
 
   SourceManager sourceMgr;
   DiagnosticEngine diags(sourceMgr);
-  Evaluator evaluator(diags, CycleDiagnosticKind::FullDiagnose);
+  Evaluator evaluator(diags);
   evaluator.registerRequestFunctions(SWIFT_ARITHMETIC_EVALUATOR_ZONE,
                                      arithmeticRequestFunctions);