Skip to content

Commit d5497f6

Browse files
authored
[Sema] Stop visiting existential exprs an extra time checking for uses (#27166)
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 (cherry picked from commit dc59cd2)
1 parent 778a530 commit d5497f6

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,12 +22,15 @@
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 "llvm/ADT/MapVector.h"
2930
#include "llvm/ADT/StringSwitch.h"
3031
#include "llvm/Support/SaveAndRestore.h"
32+
33+
#define DEBUG_TYPE "Sema"
3134
using namespace swift;
3235

3336
/// Return true if this expression is an implicit promotion from T to T?.
@@ -2045,6 +2048,10 @@ class VarDeclUsageChecker : public ASTWalker {
20452048
/// This is a mapping from VarDecls to the if/while/guard statement that they
20462049
/// occur in, when they are in a pattern in a StmtCondition.
20472050
llvm::SmallDenseMap<VarDecl*, LabeledConditionalStmt*> StmtConditionForVD;
2051+
2052+
#ifndef NDEBUG
2053+
llvm::SmallPtrSet<Expr*, 32> AllExprsSeen;
2054+
#endif
20482055

20492056
bool sawError = false;
20502057

@@ -2788,13 +2795,19 @@ void VarDeclUsageChecker::markStoredOrInOutExpr(Expr *E, unsigned Flags) {
27882795

27892796
/// The heavy lifting happens when visiting expressions.
27902797
std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
2798+
STATISTIC(VarDeclUsageCheckerExprVisits,
2799+
"# of times VarDeclUsageChecker::walkToExprPre is called");
2800+
++VarDeclUsageCheckerExprVisits;
2801+
27912802
// Sema leaves some subexpressions null, which seems really unfortunate. It
27922803
// should replace them with ErrorExpr.
27932804
if (E == nullptr || !E->getType() || E->getType()->hasError()) {
27942805
sawError = true;
27952806
return { false, E };
27962807
}
27972808

2809+
assert(AllExprsSeen.insert(E).second && "duplicate traversal");
2810+
27982811
// If this is a DeclRefExpr found in a random place, it is a load of the
27992812
// vardecl.
28002813
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
@@ -2839,9 +2852,13 @@ std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
28392852
return { false, E };
28402853
}
28412854

2842-
// If we see an OpenExistentialExpr, remember the mapping for its OpaqueValue.
2843-
if (auto *oee = dyn_cast<OpenExistentialExpr>(E))
2855+
// If we see an OpenExistentialExpr, remember the mapping for its OpaqueValue
2856+
// and only walk the subexpr.
2857+
if (auto *oee = dyn_cast<OpenExistentialExpr>(E)) {
28442858
OpaqueValueMap[oee->getOpaqueValue()] = oee->getExistentialValue();
2859+
oee->getSubExpr()->walk(*this);
2860+
return { false, E };
2861+
}
28452862

28462863
// Visit bindings.
28472864
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)