Skip to content

Commit dc59cd2

Browse files
authored
[Sema] Stop visiting existential exprs an extra time checking for uses (swiftlang#27150)
This was causing an exponential amount of time traversing the AST with deeply chained protocol extension methods, such as in the TestCodableRouter.swift test in Kitura. If the OpaqueValueExpr is referenced more than once within the OpenExistentialExpr it'll still get visited more than once, but that doesn't seem to happen in practice. If it turns out to be a problem, we can weaken the assertion I'm adding here. https://bugs.swift.org/browse/SR-11012
1 parent e0160a4 commit dc59cd2

File tree

2 files changed

+36
-2
lines changed

2 files changed

+36
-2
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,16 @@
2222
#include "swift/AST/Pattern.h"
2323
#include "swift/Basic/Defer.h"
2424
#include "swift/Basic/SourceManager.h"
25+
#include "swift/Basic/Statistic.h"
2526
#include "swift/Basic/StringExtras.h"
2627
#include "swift/Parse/Lexer.h"
2728
#include "swift/Parse/Parser.h"
2829
#include "swift/Sema/IDETypeChecking.h"
2930
#include "llvm/ADT/MapVector.h"
3031
#include "llvm/ADT/StringSwitch.h"
3132
#include "llvm/Support/SaveAndRestore.h"
33+
34+
#define DEBUG_TYPE "Sema"
3235
using namespace swift;
3336

3437
/// Return true if this expression is an implicit promotion from T to T?.
@@ -1992,6 +1995,10 @@ class VarDeclUsageChecker : public ASTWalker {
19921995
/// This is a mapping from VarDecls to the if/while/guard statement that they
19931996
/// occur in, when they are in a pattern in a StmtCondition.
19941997
llvm::SmallDenseMap<VarDecl*, LabeledConditionalStmt*> StmtConditionForVD;
1998+
1999+
#ifndef NDEBUG
2000+
llvm::SmallPtrSet<Expr*, 32> AllExprsSeen;
2001+
#endif
19952002

19962003
bool sawError = false;
19972004

@@ -2732,13 +2739,19 @@ void VarDeclUsageChecker::markStoredOrInOutExpr(Expr *E, unsigned Flags) {
27322739

27332740
/// The heavy lifting happens when visiting expressions.
27342741
std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
2742+
STATISTIC(VarDeclUsageCheckerExprVisits,
2743+
"# of times VarDeclUsageChecker::walkToExprPre is called");
2744+
++VarDeclUsageCheckerExprVisits;
2745+
27352746
// Sema leaves some subexpressions null, which seems really unfortunate. It
27362747
// should replace them with ErrorExpr.
27372748
if (E == nullptr || !E->getType() || E->getType()->hasError()) {
27382749
sawError = true;
27392750
return { false, E };
27402751
}
27412752

2753+
assert(AllExprsSeen.insert(E).second && "duplicate traversal");
2754+
27422755
// If this is a DeclRefExpr found in a random place, it is a load of the
27432756
// vardecl.
27442757
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
@@ -2783,9 +2796,13 @@ std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
27832796
return { false, E };
27842797
}
27852798

2786-
// If we see an OpenExistentialExpr, remember the mapping for its OpaqueValue.
2787-
if (auto *oee = dyn_cast<OpenExistentialExpr>(E))
2799+
// If we see an OpenExistentialExpr, remember the mapping for its OpaqueValue
2800+
// and only walk the subexpr.
2801+
if (auto *oee = dyn_cast<OpenExistentialExpr>(E)) {
27882802
OpaqueValueMap[oee->getOpaqueValue()] = oee->getExistentialValue();
2803+
oee->getSubExpr()->walk(*this);
2804+
return { false, E };
2805+
}
27892806

27902807
// Visit bindings.
27912808
if (auto ove = dyn_cast<OpaqueValueExpr>(E)) {
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %scale-test --begin 1 --end 5 --step 1 --select VarDeclUsageCheckerExprVisits %s
2+
// REQUIRES: OS=macosx
3+
// REQUIRES: asserts
4+
5+
protocol Proto {}
6+
extension Proto {
7+
var selfish: Proto { return self }
8+
}
9+
10+
struct X: Proto {}
11+
12+
func test(_ x: X) -> Proto {
13+
return x
14+
%for i in range(0, N*5):
15+
.selfish
16+
%end
17+
}

0 commit comments

Comments
 (0)