Skip to content

Commit 9e61b01

Browse files
committed
Rework computation of local discriminators for named entities.
Local discriminators for named entities are currently being set by the parser, so entities not created by the parser (e.g., that come from synthesized code) don't get local discriminators. Moreover, there is no checking to ensure that every named local entity gets a local discriminator, so some entities would incorrectly get a local discriminator of 0. Assign local discriminators as part of setting closure discriminators, in response to a request asking for the local discriminator, so the parser does not need to track this information, and all local declarations---including synthesized ones---get local discriminators. And add checking to make sure that every entity that needs a local discriminator gets assigned one. There are a few interesting cases in here: * There was a potential mangling collision with local property wrappers because their generated variables weren't getting local discriminators * $interpolation variables introduced for string interpolation weren't getting local discriminators, they were just wrong. * "Local rename" when dealing with captures like `[x]` was dependent on the new delcaration of `x` *not* getting a local discriminator. There are funny cases involving nesting where it would do the wrong thing.
1 parent 77527c9 commit 9e61b01

File tree

13 files changed

+229
-49
lines changed

13 files changed

+229
-49
lines changed

include/swift/AST/Decl.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2292,10 +2292,14 @@ class GenericParameterReferenceInfo final {
22922292
/// ValueDecl - All named decls that are values in the language. These can
22932293
/// have a type, etc.
22942294
class ValueDecl : public Decl {
2295+
public:
2296+
enum : unsigned { InvalidDiscriminator = 0xFFFF };
2297+
2298+
private:
22952299
DeclName Name;
22962300
SourceLoc NameLoc;
22972301
llvm::PointerIntPair<Type, 3, OptionalEnum<AccessLevel>> TypeAndAccess;
2298-
unsigned LocalDiscriminator = 0;
2302+
unsigned LocalDiscriminator = InvalidDiscriminator;
22992303

23002304
struct {
23012305
/// Whether the "IsObjC" bit has been computed yet.
@@ -2582,6 +2586,12 @@ class ValueDecl : public Decl {
25822586
unsigned getLocalDiscriminator() const;
25832587
void setLocalDiscriminator(unsigned index);
25842588

2589+
/// Whether this declaration has a local discriminator.
2590+
bool hasLocalDiscriminator() const;
2591+
2592+
/// Return the "raw" local discriminator, without computing it.
2593+
unsigned getRawLocalDiscriminator() const { return LocalDiscriminator; }
2594+
25852595
/// Retrieve the declaration that this declaration overrides, if any.
25862596
ValueDecl *getOverriddenDecl() const;
25872597

include/swift/AST/TypeCheckRequests.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3850,7 +3850,7 @@ class SynthesizeRuntimeMetadataAttrGeneratorBody
38503850
/// Compute the local discriminators for the given declaration context.
38513851
///
38523852
/// This is a state-changing operation for closures within the context, which
3853-
/// produces the number of assigned discriminators.
3853+
/// produces the discriminator value that any subsequent requests should use.
38543854
class LocalDiscriminatorsRequest
38553855
: public SimpleRequest<LocalDiscriminatorsRequest,
38563856
unsigned(DeclContext *),

include/swift/Parse/LocalContext.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,11 @@ namespace swift {
2727

2828
/// Information associated with parsing a local context.
2929
class LocalContext {
30-
/// A map holding the next discriminator for declarations with
31-
/// various identifiers.
32-
llvm::DenseMap<Identifier, unsigned> LocalDiscriminators;
33-
3430
LocalContext(const LocalContext &) = delete;
3531
LocalContext &operator=(const LocalContext &) = delete;
3632

3733
public:
3834
LocalContext() = default;
39-
40-
/// Return a number that'll be unique in this context across all
41-
/// declarations with the given name.
42-
unsigned claimNextNamedDiscriminator(Identifier name) {
43-
assert(!name.empty() &&
44-
"setting a local discriminator on an anonymous decl; "
45-
"maybe the name hasn't been set yet?");
46-
return LocalDiscriminators[name]++;
47-
}
4835
};
4936

5037
/// Information associated with parsing the top-level context.

lib/AST/Decl.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2696,13 +2696,49 @@ bool ValueDecl::isInstanceMember() const {
26962696
llvm_unreachable("bad DeclKind");
26972697
}
26982698

2699+
bool ValueDecl::hasLocalDiscriminator() const {
2700+
// Generic parameters and unnamed parameters never have local discriminators.
2701+
if (isa<GenericTypeParamDecl>(this) ||
2702+
(isa<ParamDecl>(this) && !hasName()))
2703+
return false;
2704+
2705+
// Implicit and unnamed declarations never have local discriminators.
2706+
if (getBaseName().isSpecial())
2707+
return false;
2708+
2709+
// If we are not in a local context, there's nothing to do.
2710+
if (!getDeclContext()->isLocalContext())
2711+
return false;
2712+
2713+
return true;
2714+
}
2715+
26992716
unsigned ValueDecl::getLocalDiscriminator() const {
2717+
// If we have already assigned a local discriminator, we're done.
2718+
if (LocalDiscriminator != InvalidDiscriminator)
2719+
return LocalDiscriminator;
2720+
2721+
// If this declaration does not have a local discriminator, use 0 as a
2722+
// stand-in.
2723+
if (!hasLocalDiscriminator())
2724+
return 0;
2725+
2726+
// Assign local discriminators in this context.
2727+
evaluateOrDefault(
2728+
getASTContext().evaluator,
2729+
LocalDiscriminatorsRequest{getDeclContext()}, InvalidDiscriminator);
2730+
if (LocalDiscriminator == InvalidDiscriminator)
2731+
dump();
2732+
2733+
assert(LocalDiscriminator != InvalidDiscriminator);
2734+
27002735
return LocalDiscriminator;
27012736
}
27022737

27032738
void ValueDecl::setLocalDiscriminator(unsigned index) {
2704-
assert(getDeclContext()->isLocalContext());
2705-
assert(LocalDiscriminator == 0 && "LocalDiscriminator is set multiple times");
2739+
assert(hasLocalDiscriminator());
2740+
assert(LocalDiscriminator == InvalidDiscriminator &&
2741+
"LocalDiscriminator is set multiple times");
27062742
LocalDiscriminator = index;
27072743
}
27082744

lib/Index/Index.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,10 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
448448
StringScratchSpace stringStorage;
449449
ContainerTracker Containers;
450450

451+
// Contains a mapping for captures of the form [x], from the declared "x"
452+
// to the captured "x" in the enclosing scope.
453+
llvm::DenseMap<VarDecl *, VarDecl *> sameNamedCaptures;
454+
451455
bool getNameAndUSR(ValueDecl *D, ExtensionDecl *ExtD,
452456
StringRef &name, StringRef &USR) {
453457
auto &result = nameAndUSRCache[ExtD ? (Decl*)ExtD : D];
@@ -701,6 +705,26 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
701705
bool walkToExprPre(Expr *E) override {
702706
if (Cancelled)
703707
return false;
708+
709+
// Record any captures of the form [x], herso we can treat
710+
if (auto captureList = dyn_cast<CaptureListExpr>(E)) {
711+
for (const auto &capture : captureList->getCaptureList()) {
712+
auto declaredVar = capture.getVar();
713+
if (capture.PBD->getEqualLoc(0).isValid())
714+
continue;
715+
716+
VarDecl *capturedVar = nullptr;
717+
if (auto init = capture.PBD->getInit(0)) {
718+
if (auto declRef = dyn_cast<DeclRefExpr>(init))
719+
capturedVar = dyn_cast_or_null<VarDecl>(declRef->getDecl());
720+
}
721+
722+
if (capturedVar) {
723+
sameNamedCaptures[declaredVar] = capturedVar;
724+
}
725+
}
726+
}
727+
704728
ExprStack.push_back(E);
705729
Containers.activateContainersFor(E);
706730
handleMemberwiseInitRefs(E);
@@ -1611,6 +1635,15 @@ bool IndexSwiftASTWalker::initIndexSymbol(ValueDecl *D, SourceLoc Loc,
16111635
if (auto *VD = dyn_cast<VarDecl>(D)) {
16121636
// Always base the symbol information on the canonical VarDecl
16131637
D = VD->getCanonicalVarDecl();
1638+
1639+
// Dig back to the original captured variable.
1640+
while (true) {
1641+
auto captured = sameNamedCaptures.find(cast<VarDecl>(D));
1642+
if (captured == sameNamedCaptures.end())
1643+
break;
1644+
1645+
D = captured->second;
1646+
}
16141647
}
16151648

16161649
Info.decl = D;

lib/Parse/ParseDecl.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4620,10 +4620,6 @@ void Parser::setLocalDiscriminator(ValueDecl *D) {
46204620
if (auto TD = dyn_cast<TypeDecl>(D))
46214621
if (!InInactiveClauseEnvironment)
46224622
SF.LocalTypeDecls.insert(TD);
4623-
4624-
const Identifier name = D->getBaseIdentifier();
4625-
unsigned discriminator = CurLocalContext->claimNextNamedDiscriminator(name);
4626-
D->setLocalDiscriminator(discriminator);
46274623
}
46284624

46294625
void Parser::setLocalDiscriminatorToParamList(ParameterList *PL) {

lib/Parse/Parser.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/AST/ParseRequests.h"
2323
#include "swift/AST/PrettyStackTrace.h"
2424
#include "swift/AST/SourceFile.h"
25+
#include "swift/AST/TypeCheckRequests.h"
2526
#include "swift/Basic/Defer.h"
2627
#include "swift/Basic/SourceManager.h"
2728
#include "swift/Parse/Lexer.h"
@@ -149,6 +150,10 @@ void Parser::performIDEInspectionSecondPassImpl(
149150

150151
DeclContext *DC = info.ParentContext;
151152

153+
// Forget about the fact that we may have already computed local
154+
// discriminators.
155+
Context.evaluator.clearCachedOutput(LocalDiscriminatorsRequest{DC});
156+
152157
switch (info.Kind) {
153158
case IDEInspectionDelayedDeclKind::TopLevelCodeDecl: {
154159
// Re-enter the top-level code decl context.

0 commit comments

Comments
 (0)