swiftlang · slavapestov · Apr 23, 2019 · Apr 23, 2019 · Apr 23, 2019 · Apr 23, 2019
diff --git a/include/swift/AST/Evaluator.h b/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);

diff --git a/include/swift/Basic/CycleDiagnosticKind.h b/include/swift/Basic/CycleDiagnosticKind.h
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.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.

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
@@ -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)),

diff --git a/lib/AST/ASTScope.cpp b/lib/AST/ASTScope.cpp
@@ -877,8 +877,14 @@ ASTScope *ASTScope::createIfNeeded(const ASTScope *parent, Decl *decl) {
 
     // If we already have a scope of the (possible) generic parameters,
     // add the body.
-    if (parent->getKind() == ASTScopeKind::ExtensionGenericParams)
+    if (parent->getKind() == ASTScopeKind::ExtensionGenericParams) {
+      // If there is no body because we have an invalid extension written
+      // without braces in the source, just return.
+      if (ext->getBraces().Start == ext->getBraces().End)
+        return nullptr;
+
       return new (ctx) ASTScope(parent, cast<IterableDeclContext>(ext));
+    }
 
     // Otherwise, form the extension's generic parameters scope.
     return new (ctx) ASTScope(parent, ext);

diff --git a/lib/AST/Evaluator.cpp b/lib/AST/Evaluator.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;

diff --git a/lib/AST/UnqualifiedLookup.cpp b/lib/AST/UnqualifiedLookup.cpp
@@ -102,7 +102,9 @@ template <typename D> bool shouldLookupMembers(D *decl, SourceLoc loc) {
 
   // Within the braces, always look for members.
   auto &ctx = decl->getASTContext();
-  if (ctx.SourceMgr.rangeContainsTokenLoc(decl->getBraces(), loc))
+  auto braces = decl->getBraces();
+  if (braces.Start != braces.End &&
+      ctx.SourceMgr.rangeContainsTokenLoc(braces, loc))
     return true;
 
   // Within 'where' clause, we can also look for members.

diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.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();

diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp
@@ -1500,7 +1500,8 @@ static ObjCSelector inferObjCName(ValueDecl *decl) {
                             attr->AtLoc,
                             diag::objc_override_method_selector_mismatch,
                             *attr->getName(), overriddenSelector);
-              fixDeclarationObjCName(diag, decl, overriddenSelector);
+              fixDeclarationObjCName(diag, decl, attr->getName(),
+                                     overriddenSelector);
             }
 
             overriddenFunc->diagnose(diag::overridden_here);
@@ -1581,7 +1582,9 @@ static ObjCSelector inferObjCName(ValueDecl *decl) {
                                    req->getFullName(),
                                    proto->getFullName(),
                                    *req->getObjCRuntimeName());
-        fixDeclarationObjCName(diag, decl, req->getObjCRuntimeName());
+        fixDeclarationObjCName(diag, decl,
+                               decl->getObjCRuntimeName(/*skipIsObjC=*/true),
+                               req->getObjCRuntimeName());
       };
       diagnoseCandidate(firstReq);
       diagnoseCandidate(req);
@@ -1874,6 +1877,7 @@ bool swift::fixDeclarationName(InFlightDiagnostic &diag, ValueDecl *decl,
 }
 
 bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
+                                   Optional<ObjCSelector> nameOpt,
                                    Optional<ObjCSelector> targetNameOpt,
                                    bool ignoreImpliedName) {
   if (decl->isImplicit())
@@ -1886,8 +1890,7 @@ bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
     return false;
   }
 
-  // Determine the Objective-C name of the declaration.
-  ObjCSelector name = *decl->getObjCRuntimeName();
+  auto name = *nameOpt;
   auto targetName = *targetNameOpt;
 
   // Dig out the existing '@objc' attribute on the witness. We don't care
@@ -2336,7 +2339,9 @@ bool swift::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) {
 
       // Fix the '@objc' attribute, if needed.
       if (!conflicts[0]->canInferObjCFromRequirement(req))
-        fixDeclarationObjCName(diag, conflicts[0], req->getObjCRuntimeName(),
+        fixDeclarationObjCName(diag, conflicts[0],
+                               conflicts[0]->getObjCRuntimeName(),
+                               req->getObjCRuntimeName(),
                                /*ignoreImpliedName=*/true);
     }
 

diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -330,7 +330,9 @@ static bool checkObjCWitnessSelector(TypeChecker &tc, ValueDecl *req,
                             diagInfo.first, diagInfo.second,
                             witnessFunc->getObjCSelector(),
                             reqFunc->getObjCSelector());
-    fixDeclarationObjCName(diag, witnessFunc, reqFunc->getObjCSelector());
+    fixDeclarationObjCName(diag, witnessFunc,
+                           witnessFunc->getObjCSelector(),
+                           reqFunc->getObjCSelector());
 
     return true;
   }
@@ -2112,7 +2114,9 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
 
     // Also fix the Objective-C name, if needed.
     if (!match.Witness->canInferObjCFromRequirement(req))
-      fixDeclarationObjCName(diag, match.Witness, req->getObjCRuntimeName());
+      fixDeclarationObjCName(diag, match.Witness,
+                             match.Witness->getObjCRuntimeName(),
+                             req->getObjCRuntimeName());
     break;
   }
 
@@ -3830,7 +3834,8 @@ void ConformanceChecker::resolveValueWitnesses() {
             if (!witness->canInferObjCFromRequirement(requirement)) {
               fixDeclarationObjCName(
                   fixItDiag.getValue(), witness,
-                  cast<AbstractFunctionDecl>(requirement)->getObjCSelector());
+                  witness->getObjCRuntimeName(),
+                  requirement->getObjCRuntimeName());
             }
           } else if (isa<VarDecl>(witness)) {
             Optional<InFlightDiagnostic> fixItDiag =
@@ -3850,10 +3855,9 @@ void ConformanceChecker::resolveValueWitnesses() {
             }
             if (!witness->canInferObjCFromRequirement(requirement)) {
               fixDeclarationObjCName(
-                 fixItDiag.getValue(), witness,
-                 ObjCSelector(requirement->getASTContext(), 0,
-                              cast<VarDecl>(requirement)
-                                ->getObjCPropertyName()));
+                  fixItDiag.getValue(), witness,
+                  witness->getObjCRuntimeName(),
+                  requirement->getObjCRuntimeName());
             }
           } else if (isa<SubscriptDecl>(witness)) {
             Optional<InFlightDiagnostic> fixItDiag =
@@ -4799,7 +4803,9 @@ static void diagnosePotentialWitness(TypeChecker &tc,
     auto diag = tc.diagnose(witness,
                             diag::optional_req_nonobjc_near_match_add_objc);
     if (!witness->canInferObjCFromRequirement(req))
-      fixDeclarationObjCName(diag, witness, req->getObjCRuntimeName());
+      fixDeclarationObjCName(diag, witness,
+                             witness->getObjCRuntimeName(),
+                             req->getObjCRuntimeName());
   } else {
     diagnoseMatch(conformance->getDeclContext()->getParentModule(),
                   conformance, req, match);

diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -2228,6 +2228,7 @@ bool fixDeclarationName(InFlightDiagnostic &diag, ValueDecl *decl,
 /// For properties, the selector should be a zero-parameter selector of the
 /// given property's name.
 bool fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
+                            Optional<ObjCSelector> nameOpt,
                             Optional<ObjCSelector> targetNameOpt,
                             bool ignoreImpliedName = false);
 

diff --git a/test/decl/class/circular_inheritance.swift b/test/decl/class/circular_inheritance.swift
@@ -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> {}
 }
 

diff --git a/unittests/AST/ArithmeticEvaluator.cpp b/unittests/AST/ArithmeticEvaluator.cpp
@@ -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);