Skip to content

More ASTScope fixes in preparation for turning off parse-time lookup #34104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 29, 2020
Merged

More ASTScope fixes in preparation for turning off parse-time lookup #34104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe9d1ed
Sema: Use ASTScope::lookupSingleLocalDecl() in VarDeclUsageChecker::h…
slavapestov Sep 26, 2020
025921c
Parse: Move the lookup of 'self' for a SuperRefExpr to preCheckExpres…
slavapestov Sep 28, 2020
bd7763d
AST: Remove unused UnqualifiedLookupFactory::resultsSizeBeforeLocalsPass
slavapestov Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/swift/AST/Expr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,7 @@ class SuperRefExpr : public Expr {
: Expr(ExprKind::SuperRef, Implicit, SuperTy), Self(Self), Loc(Loc) {}

VarDecl *getSelf() const { return Self; }
void setSelf(VarDecl *self) { Self = self; }

SourceLoc getSuperLoc() const { return Loc; }
SourceRange getSourceRange() const { return Loc; }
Expand Down
9 changes: 0 additions & 9 deletions lib/AST/UnqualifiedLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ namespace {
static const unsigned targetLookup;
#endif

public: // for exp debugging
unsigned resultsSizeBeforeLocalsPass = ~0;

public:
// clang-format off
UnqualifiedLookupFactory(DeclNameRef Name,
Expand Down Expand Up @@ -708,16 +705,10 @@ void UnqualifiedLookupFactory::printScopes(raw_ostream &out) const {

void UnqualifiedLookupFactory::printResults(raw_ostream &out) const {
for (auto i : indices(Results)) {
if (i == resultsSizeBeforeLocalsPass)
out << "============== next pass ============\n";
out << i << ": ";
Results[i].print(out);
out << "\n";
}
if (resultsSizeBeforeLocalsPass == Results.size())
out << "============== next pass ============\n";
if (resultsSizeBeforeLocalsPass == ~0u)
out << "never tried locals\n\n";
Comment on lines -717 to -720
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the block at 712 were useful for my debugging. But if you don't need them, it's fine by me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted it because resultsSizeBeforeLocalsPass was never written to, so reading from it always produced the value ~0U. It appears that the only usage of this variable was in the DeclContext-based unqualified lookup that I deleted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good move!

}

void UnqualifiedLookupFactory::print(raw_ostream &OS) const {
Expand Down
31 changes: 2 additions & 29 deletions lib/Parse/ParseExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,29 +853,6 @@ UnresolvedDeclRefExpr *Parser::parseExprOperator() {
return new (Context) UnresolvedDeclRefExpr(name, refKind, DeclNameLoc(loc));
}

static VarDecl *getImplicitSelfDeclForSuperContext(Parser &P,
DeclContext *DC,
SourceLoc Loc) {
auto *methodContext = DC->getInnermostMethodContext();
if (!methodContext) {
P.diagnose(Loc, diag::super_not_in_class_method);
return nullptr;
}

// Do an actual lookup for 'self' in case it shows up in a capture list.
auto *methodSelf = methodContext->getImplicitSelfDecl();
auto *lookupSelf = P.lookupInScope(DeclNameRef(P.Context.Id_self));
if (lookupSelf && lookupSelf != methodSelf) {
// FIXME: This is the wrong diagnostic for if someone manually declares a
// variable named 'self' using backticks.
P.diagnose(Loc, diag::super_in_closure_with_capture);
P.diagnose(lookupSelf->getLoc(), diag::super_in_closure_with_capture_here);
return nullptr;
}

return methodSelf;
}

/// parseExprSuper
///
/// expr-super:
Expand Down Expand Up @@ -903,12 +880,8 @@ ParserResult<Expr> Parser::parseExprSuper() {
return nullptr;
}

VarDecl *selfDecl =
getImplicitSelfDeclForSuperContext(*this, CurDeclContext, superLoc);
if (!selfDecl)
return makeParserResult(new (Context) ErrorExpr(superLoc));

return makeParserResult(new (Context) SuperRefExpr(selfDecl, superLoc,
return makeParserResult(new (Context) SuperRefExpr(/*selfDecl=*/nullptr,
superLoc,
/*Implicit=*/false));
}

Expand Down
24 changes: 20 additions & 4 deletions lib/Sema/MiscDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2282,6 +2282,8 @@ bool swift::computeFixitsForOverridenDeclaration(
namespace {

class VarDeclUsageChecker : public ASTWalker {
DeclContext *DC;

DiagnosticEngine &Diags;
// Keep track of some information about a variable.
enum {
Expand Down Expand Up @@ -2318,7 +2320,8 @@ class VarDeclUsageChecker : public ASTWalker {
void operator=(const VarDeclUsageChecker &) = delete;

public:
VarDeclUsageChecker(DiagnosticEngine &Diags) : Diags(Diags) {}
VarDeclUsageChecker(DeclContext *DC,
DiagnosticEngine &Diags) : DC(DC), Diags(Diags) {}

// After we have scanned the entire region, diagnose variables that could be
// declared with a narrower usage kind.
Expand Down Expand Up @@ -3140,14 +3143,27 @@ std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
void VarDeclUsageChecker::handleIfConfig(IfConfigDecl *ICD) {
struct ConservativeDeclMarker : public ASTWalker {
VarDeclUsageChecker &VDUC;
ConservativeDeclMarker(VarDeclUsageChecker &VDUC) : VDUC(VDUC) {}
SourceFile *SF;

ConservativeDeclMarker(VarDeclUsageChecker &VDUC)
: VDUC(VDUC), SF(VDUC.DC->getParentSourceFile()) {}

Expr *walkToExprPost(Expr *E) override {
// If we see a bound reference to a decl in an inactive #if block, then
// conservatively mark it read and written. This will silence "variable
// unused" and "could be marked let" warnings for it.
if (auto *DRE = dyn_cast<DeclRefExpr>(E))
VDUC.addMark(DRE->getDecl(), RK_Read|RK_Written);
else if (auto *declRef = dyn_cast<UnresolvedDeclRefExpr>(E)) {
auto name = declRef->getName();
auto loc = declRef->getLoc();
if (name.isSimpleName() && loc.isValid()) {
auto *varDecl = dyn_cast_or_null<VarDecl>(
ASTScope::lookupSingleLocalDecl(SF, name.getFullName(), loc));
if (varDecl)
VDUC.addMark(varDecl, RK_Read|RK_Written);
}
}
return E;
}
};
Expand All @@ -3166,7 +3182,7 @@ void VarDeclUsageChecker::handleIfConfig(IfConfigDecl *ICD) {
void swift::
performTopLevelDeclDiagnostics(TopLevelCodeDecl *TLCD) {
auto &ctx = TLCD->getDeclContext()->getASTContext();
VarDeclUsageChecker checker(ctx.Diags);
VarDeclUsageChecker checker(TLCD, ctx.Diags);
TLCD->walk(checker);
}

Expand All @@ -3181,7 +3197,7 @@ void swift::performAbstractFuncDeclDiagnostics(AbstractFunctionDecl *AFD) {
// be checked as part of their parent function or TopLevelCodeDecl.
if (!AFD->getDeclContext()->isLocalContext()) {
auto &ctx = AFD->getDeclContext()->getASTContext();
VarDeclUsageChecker checker(ctx.Diags);
VarDeclUsageChecker checker(AFD, ctx.Diags);
AFD->walk(checker);
}

Expand Down
6 changes: 4 additions & 2 deletions lib/Sema/TypeCheckCaptures.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,8 +572,10 @@ class FindCapturedVars : public ASTWalker {

// When we see a reference to the 'super' expression, capture 'self' decl.
if (auto *superE = dyn_cast<SuperRefExpr>(E)) {
if (CurDC->isChildContextOf(superE->getSelf()->getDeclContext()))
addCapture(CapturedValue(superE->getSelf(), 0, superE->getLoc()));
if (auto *selfDecl = superE->getSelf()) {
if (CurDC->isChildContextOf(selfDecl->getDeclContext()))
addCapture(CapturedValue(selfDecl, 0, superE->getLoc()));
}
return { false, superE };
}

Expand Down
34 changes: 34 additions & 0 deletions lib/Sema/TypeCheckConstraints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,29 @@ namespace {

bool shouldWalkCaptureInitializerExpressions() override { return true; }

VarDecl *getImplicitSelfDeclForSuperContext(SourceLoc Loc) {
auto *methodContext = DC->getInnermostMethodContext();
if (!methodContext) {
Ctx.Diags.diagnose(Loc, diag::super_not_in_class_method);
return nullptr;
}

// Do an actual lookup for 'self' in case it shows up in a capture list.
auto *methodSelf = methodContext->getImplicitSelfDecl();
auto *lookupSelf = ASTScope::lookupSingleLocalDecl(DC->getParentSourceFile(),
Ctx.Id_self, Loc);
if (lookupSelf && lookupSelf != methodSelf) {
// FIXME: This is the wrong diagnostic for if someone manually declares a
// variable named 'self' using backticks.
Ctx.Diags.diagnose(Loc, diag::super_in_closure_with_capture);
Ctx.Diags.diagnose(lookupSelf->getLoc(),
diag::super_in_closure_with_capture_here);
return nullptr;
}

return methodSelf;
}

std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
// If this is a call, record the argument expression.
if (auto call = dyn_cast<ApplyExpr>(expr)) {
Expand Down Expand Up @@ -1156,6 +1179,17 @@ namespace {
return std::make_pair(recursive, expr);
};

// Resolve 'super' references.
if (auto *superRef = dyn_cast<SuperRefExpr>(expr)) {
auto loc = superRef->getLoc();
auto *selfDecl = getImplicitSelfDeclForSuperContext(loc);
if (selfDecl == nullptr)
return finish(true, new (Ctx) ErrorExpr(loc));

superRef->setSelf(selfDecl);
return finish(true, superRef);
}

// For closures, type-check the patterns and result type as written,
// but do not walk into the body. That will be type-checked after
// we've determine the complete function type.
Expand Down