Skip to content

Commit e303cea

Browse files
slavapestovslavapestov
authored
Merge pull request #24213 from slavapestov/diagnose-cycles
Always diagnose request evaluator cycles
2 parents afc9e20 + 3f5a06b commit e303cea

File tree

13 files changed

+57
-84
lines changed

13 files changed

+57
-84
lines changed

‎include/swift/AST/Evaluator.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
#include "swift/AST/AnyRequest.h"
2222
#include "swift/Basic/AnyValue.h"
23-
#include "swift/Basic/CycleDiagnosticKind.h"
2423
#include "swift/Basic/Defer.h"
2524
#include "swift/Basic/Statistic.h"
2625
#include "llvm/ADT/DenseMap.h"
@@ -184,8 +183,8 @@ class Evaluator {
184183
/// diagnostics will be emitted.
185184
DiagnosticEngine &diags;
186185

187-
/// Whether to diagnose cycles or ignore them completely.
188-
CycleDiagnosticKind shouldDiagnoseCycles;
186+
/// Whether to dump detailed debug info for cycles.
187+
bool debugDumpCycles;
189188

190189
/// Used to report statistics about which requests were evaluated, if
191190
/// non-null.
@@ -237,7 +236,7 @@ class Evaluator {
237236
public:
238237
/// Construct a new evaluator that can emit cyclic-dependency
239238
/// diagnostics through the given diagnostics engine.
240-
Evaluator(DiagnosticEngine &diags, CycleDiagnosticKind shouldDiagnoseCycles);
239+
Evaluator(DiagnosticEngine &diags, bool debugDumpCycles=false);
241240

242241
/// Emit GraphViz output visualizing the request graph.
243242
void emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath);

‎include/swift/Basic/CycleDiagnosticKind.h

Lines changed: 0 additions & 34 deletions
This file was deleted.

‎include/swift/Basic/LangOptions.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#define SWIFT_BASIC_LANGOPTIONS_H
2020

2121
#include "swift/Config.h"
22-
#include "swift/Basic/CycleDiagnosticKind.h"
2322
#include "swift/Basic/LLVM.h"
2423
#include "swift/Basic/Version.h"
2524
#include "llvm/ADT/ArrayRef.h"
@@ -185,9 +184,8 @@ namespace swift {
185184
/// This is for testing purposes.
186185
std::string DebugForbidTypecheckPrefix;
187186

188-
/// How to diagnose cycles encountered
189-
CycleDiagnosticKind EvaluatorCycleDiagnostics =
190-
CycleDiagnosticKind::NoDiagnose;
187+
/// Whether to dump debug info for request evaluator cycles.
188+
bool DebugDumpCycles = false;
191189

192190
/// The path to which we should emit GraphViz output for the complete
193191
/// request-evaluator graph.

‎lib/AST/ASTContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ ASTContext::ASTContext(LangOptions &langOpts, SearchPathOptions &SearchPathOpts,
479479
SearchPathOpts(SearchPathOpts),
480480
SourceMgr(SourceMgr),
481481
Diags(Diags),
482-
evaluator(Diags, langOpts.EvaluatorCycleDiagnostics),
482+
evaluator(Diags, langOpts.DebugDumpCycles),
483483
TheBuiltinModule(createBuiltinModule(*this)),
484484
StdlibModuleName(getIdentifier(STDLIB_NAME)),
485485
SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),

‎lib/AST/ASTScope.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,8 +877,14 @@ ASTScope *ASTScope::createIfNeeded(const ASTScope *parent, Decl *decl) {
877877

878878
// If we already have a scope of the (possible) generic parameters,
879879
// add the body.
880-
if (parent->getKind() == ASTScopeKind::ExtensionGenericParams)
880+
if (parent->getKind() == ASTScopeKind::ExtensionGenericParams) {
881+
// If there is no body because we have an invalid extension written
882+
// without braces in the source, just return.
883+
if (ext->getBraces().Start == ext->getBraces().End)
884+
return nullptr;
885+
881886
return new (ctx) ASTScope(parent, cast<IterableDeclContext>(ext));
887+
}
882888

883889
// Otherwise, form the extension's generic parameters scope.
884890
return new (ctx) ASTScope(parent, ext);

‎lib/AST/Evaluator.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,8 @@ void Evaluator::registerRequestFunctions(
5959
requestFunctionsByZone.push_back({zoneID, functions});
6060
}
6161

62-
Evaluator::Evaluator(DiagnosticEngine &diags,
63-
CycleDiagnosticKind shouldDiagnoseCycles)
64-
: diags(diags), shouldDiagnoseCycles(shouldDiagnoseCycles) { }
62+
Evaluator::Evaluator(DiagnosticEngine &diags, bool debugDumpCycles)
63+
: diags(diags), debugDumpCycles(debugDumpCycles) { }
6564

6665
void Evaluator::emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath) {
6766
std::error_code error;
@@ -80,24 +79,16 @@ bool Evaluator::checkDependency(const AnyRequest &request) {
8079
}
8180

8281
// Diagnose cycle.
83-
switch (shouldDiagnoseCycles) {
84-
case CycleDiagnosticKind::NoDiagnose:
85-
return true;
82+
diagnoseCycle(request);
8683

87-
case CycleDiagnosticKind::DebugDiagnose: {
84+
if (debugDumpCycles) {
8885
llvm::errs() << "===CYCLE DETECTED===\n";
8986
llvm::DenseSet<AnyRequest> visitedAnywhere;
9087
llvm::SmallVector<AnyRequest, 4> visitedAlongPath;
9188
std::string prefixStr;
9289
printDependencies(activeRequests.front(), llvm::errs(), visitedAnywhere,
9390
visitedAlongPath, activeRequests.getArrayRef(),
9491
prefixStr, /*lastChild=*/true);
95-
return true;
96-
}
97-
98-
case CycleDiagnosticKind::FullDiagnose:
99-
diagnoseCycle(request);
100-
return true;
10192
}
10293

10394
return true;

‎lib/AST/UnqualifiedLookup.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ template <typename D> bool shouldLookupMembers(D *decl, SourceLoc loc) {
102102

103103
// Within the braces, always look for members.
104104
auto &ctx = decl->getASTContext();
105-
if (ctx.SourceMgr.rangeContainsTokenLoc(decl->getBraces(), loc))
105+
auto braces = decl->getBraces();
106+
if (braces.Start != braces.End &&
107+
ctx.SourceMgr.rangeContainsTokenLoc(braces, loc))
106108
return true;
107109

108110
// Within 'where' clause, we can also look for members.

‎lib/Frontend/CompilerInvocation.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
360360
Opts.DebugForbidTypecheckPrefix = A->getValue();
361361
}
362362

363-
if (Args.getLastArg(OPT_debug_cycles)) {
364-
Opts.EvaluatorCycleDiagnostics = CycleDiagnosticKind::DebugDiagnose;
365-
}
363+
if (Args.getLastArg(OPT_debug_cycles))
364+
Opts.DebugDumpCycles = true;
366365

367366
if (const Arg *A = Args.getLastArg(OPT_output_request_graphviz)) {
368367
Opts.RequestEvaluatorGraphVizPath = A->getValue();

‎lib/Sema/TypeCheckDeclObjC.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,7 +1500,8 @@ static ObjCSelector inferObjCName(ValueDecl *decl) {
15001500
attr->AtLoc,
15011501
diag::objc_override_method_selector_mismatch,
15021502
*attr->getName(), overriddenSelector);
1503-
fixDeclarationObjCName(diag, decl, overriddenSelector);
1503+
fixDeclarationObjCName(diag, decl, attr->getName(),
1504+
overriddenSelector);
15041505
}
15051506

15061507
overriddenFunc->diagnose(diag::overridden_here);
@@ -1581,7 +1582,9 @@ static ObjCSelector inferObjCName(ValueDecl *decl) {
15811582
req->getFullName(),
15821583
proto->getFullName(),
15831584
*req->getObjCRuntimeName());
1584-
fixDeclarationObjCName(diag, decl, req->getObjCRuntimeName());
1585+
fixDeclarationObjCName(diag, decl,
1586+
decl->getObjCRuntimeName(/*skipIsObjC=*/true),
1587+
req->getObjCRuntimeName());
15851588
};
15861589
diagnoseCandidate(firstReq);
15871590
diagnoseCandidate(req);
@@ -1874,6 +1877,7 @@ bool swift::fixDeclarationName(InFlightDiagnostic &diag, ValueDecl *decl,
18741877
}
18751878

18761879
bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
1880+
Optional<ObjCSelector> nameOpt,
18771881
Optional<ObjCSelector> targetNameOpt,
18781882
bool ignoreImpliedName) {
18791883
if (decl->isImplicit())
@@ -1886,8 +1890,7 @@ bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
18861890
return false;
18871891
}
18881892

1889-
// Determine the Objective-C name of the declaration.
1890-
ObjCSelector name = *decl->getObjCRuntimeName();
1893+
auto name = *nameOpt;
18911894
auto targetName = *targetNameOpt;
18921895

18931896
// Dig out the existing '@objc' attribute on the witness. We don't care
@@ -2336,7 +2339,9 @@ bool swift::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) {
23362339

23372340
// Fix the '@objc' attribute, if needed.
23382341
if (!conflicts[0]->canInferObjCFromRequirement(req))
2339-
fixDeclarationObjCName(diag, conflicts[0], req->getObjCRuntimeName(),
2342+
fixDeclarationObjCName(diag, conflicts[0],
2343+
conflicts[0]->getObjCRuntimeName(),
2344+
req->getObjCRuntimeName(),
23402345
/*ignoreImpliedName=*/true);
23412346
}
23422347

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,9 @@ static bool checkObjCWitnessSelector(TypeChecker &tc, ValueDecl *req,
330330
diagInfo.first, diagInfo.second,
331331
witnessFunc->getObjCSelector(),
332332
reqFunc->getObjCSelector());
333-
fixDeclarationObjCName(diag, witnessFunc, reqFunc->getObjCSelector());
333+
fixDeclarationObjCName(diag, witnessFunc,
334+
witnessFunc->getObjCSelector(),
335+
reqFunc->getObjCSelector());
334336

335337
return true;
336338
}
@@ -2112,7 +2114,9 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
21122114

21132115
// Also fix the Objective-C name, if needed.
21142116
if (!match.Witness->canInferObjCFromRequirement(req))
2115-
fixDeclarationObjCName(diag, match.Witness, req->getObjCRuntimeName());
2117+
fixDeclarationObjCName(diag, match.Witness,
2118+
match.Witness->getObjCRuntimeName(),
2119+
req->getObjCRuntimeName());
21162120
break;
21172121
}
21182122

@@ -3830,7 +3834,8 @@ void ConformanceChecker::resolveValueWitnesses() {
38303834
if (!witness->canInferObjCFromRequirement(requirement)) {
38313835
fixDeclarationObjCName(
38323836
fixItDiag.getValue(), witness,
3833-
cast<AbstractFunctionDecl>(requirement)->getObjCSelector());
3837+
witness->getObjCRuntimeName(),
3838+
requirement->getObjCRuntimeName());
38343839
}
38353840
} else if (isa<VarDecl>(witness)) {
38363841
Optional<InFlightDiagnostic> fixItDiag =
@@ -3850,10 +3855,9 @@ void ConformanceChecker::resolveValueWitnesses() {
38503855
}
38513856
if (!witness->canInferObjCFromRequirement(requirement)) {
38523857
fixDeclarationObjCName(
3853-
fixItDiag.getValue(), witness,
3854-
ObjCSelector(requirement->getASTContext(), 0,
3855-
cast<VarDecl>(requirement)
3856-
->getObjCPropertyName()));
3858+
fixItDiag.getValue(), witness,
3859+
witness->getObjCRuntimeName(),
3860+
requirement->getObjCRuntimeName());
38573861
}
38583862
} else if (isa<SubscriptDecl>(witness)) {
38593863
Optional<InFlightDiagnostic> fixItDiag =
@@ -4799,7 +4803,9 @@ static void diagnosePotentialWitness(TypeChecker &tc,
47994803
auto diag = tc.diagnose(witness,
48004804
diag::optional_req_nonobjc_near_match_add_objc);
48014805
if (!witness->canInferObjCFromRequirement(req))
4802-
fixDeclarationObjCName(diag, witness, req->getObjCRuntimeName());
4806+
fixDeclarationObjCName(diag, witness,
4807+
witness->getObjCRuntimeName(),
4808+
req->getObjCRuntimeName());
48034809
} else {
48044810
diagnoseMatch(conformance->getDeclContext()->getParentModule(),
48054811
conformance, req, match);

‎lib/Sema/TypeChecker.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2228,6 +2228,7 @@ bool fixDeclarationName(InFlightDiagnostic &diag, ValueDecl *decl,
22282228
/// For properties, the selector should be a zero-parameter selector of the
22292229
/// given property's name.
22302230
bool fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
2231+
Optional<ObjCSelector> nameOpt,
22312232
Optional<ObjCSelector> targetNameOpt,
22322233
bool ignoreImpliedName = false);
22332234

‎test/decl/class/circular_inheritance.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
// Check that we produced superclass type requests.
99
// RUN: %{python} %utils/process-stats-dir.py --evaluate 'SuperclassTypeRequest == 17' %t/stats-dir
1010

11-
class Left
12-
: Right.Hand {
11+
class Left // expected-error {{circular reference}}
12+
: Right.Hand { // expected-note {{through reference here}}
1313
class Hand {}
1414
}
1515

16-
class Right
17-
: Left.Hand {
16+
class Right // expected-note {{through reference here}}
17+
: Left.Hand { // expected-note {{through reference here}}
1818
class Hand {}
1919
}
2020

@@ -35,14 +35,14 @@ class Outer {
3535
class Inner : Outer {}
3636
}
3737

38-
class Outer2
39-
: Outer2.Inner {
38+
class Outer2 // expected-error {{circular reference}}
39+
: Outer2.Inner { // expected-note {{through reference here}}
4040

4141
class Inner {}
4242
}
4343

44-
class Outer3
45-
: Outer3.Inner<Int> {
44+
class Outer3 // expected-error {{circular reference}}
45+
: Outer3.Inner<Int> { // expected-note {{through reference here}}
4646
class Inner<T> {}
4747
}
4848

‎unittests/AST/ArithmeticEvaluator.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ TEST(ArithmeticEvaluator, Simple) {
211211

212212
SourceManager sourceMgr;
213213
DiagnosticEngine diags(sourceMgr);
214-
Evaluator evaluator(diags, CycleDiagnosticKind::FullDiagnose);
214+
Evaluator evaluator(diags);
215215
evaluator.registerRequestFunctions(SWIFT_ARITHMETIC_EVALUATOR_ZONE,
216216
arithmeticRequestFunctions);
217217

@@ -334,7 +334,7 @@ TEST(ArithmeticEvaluator, Cycle) {
334334

335335
SourceManager sourceMgr;
336336
DiagnosticEngine diags(sourceMgr);
337-
Evaluator evaluator(diags, CycleDiagnosticKind::FullDiagnose);
337+
Evaluator evaluator(diags);
338338
evaluator.registerRequestFunctions(SWIFT_ARITHMETIC_EVALUATOR_ZONE,
339339
arithmeticRequestFunctions);
340340

0 commit comments

Comments
 (0)