Skip to content

Commit 9dcd0f9

Browse files
rintarorintaro
authored
Merge pull request #16130 from rintaro/parse-var-discriminator
[Parse] Discriminate local variables
2 parents 6d7fe7f + a6f7a8e commit 9dcd0f9

22 files changed

+574
-43
lines changed

‎include/swift/AST/Decl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,6 +2150,7 @@ class ValueDecl : public Decl {
21502150
DeclName Name;
21512151
SourceLoc NameLoc;
21522152
llvm::PointerIntPair<Type, 3, OptionalEnum<AccessLevel>> TypeAndAccess;
2153+
unsigned LocalDiscriminator = 0;
21532154

21542155
private:
21552156
bool isUsableFromInline() const;

‎include/swift/Parse/Parser.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,7 @@ class Parser {
761761
ParserStatus parseLineDirective(bool isLine = false);
762762

763763
void setLocalDiscriminator(ValueDecl *D);
764+
void setLocalDiscriminatorToParamList(ParameterList *PL);
764765

765766
/// Parse the optional attributes before a declaration.
766767
bool parseDeclAttributeList(DeclAttributes &Attributes,

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
5555
/// describe what change you made. The content of this comment isn't important;
5656
/// it just ensures a conflict if two people change the module format.
5757
/// Don't worry about adhering to the 80-column limit for this line.
58-
const uint16_t VERSION_MINOR = 418; // Last change: add begin_access [builtin].
58+
const uint16_t VERSION_MINOR = 419; // Last change: Remove discriminator from LocalDeclTableInfo.
5959

6060
using DeclIDField = BCFixed<31>;
6161

‎lib/AST/ASTContext.cpp

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,6 @@ FOR_KNOWN_FOUNDATION_TYPES(CACHE_FOUNDATION_DECL)
226226
/// \brief Map from Swift declarations to brief comments.
227227
llvm::DenseMap<const Decl *, StringRef> BriefComments;
228228

229-
/// \brief Map from local declarations to their discriminators.
230-
/// Missing entries implicitly have value 0.
231-
llvm::DenseMap<const ValueDecl *, unsigned> LocalDiscriminators;
232-
233229
/// \brief Map from declarations to foreign error conventions.
234230
/// This applies to both actual imported functions and to @objc functions.
235231
llvm::DenseMap<const AbstractFunctionDecl *,
@@ -1796,24 +1792,6 @@ void ASTContext::setBriefComment(const Decl *D, StringRef Comment) {
17961792
getImpl().BriefComments[D] = Comment;
17971793
}
17981794

1799-
unsigned ValueDecl::getLocalDiscriminator() const {
1800-
assert(getDeclContext()->isLocalContext());
1801-
auto &discriminators = getASTContext().getImpl().LocalDiscriminators;
1802-
auto it = discriminators.find(this);
1803-
if (it == discriminators.end())
1804-
return 0;
1805-
return it->second;
1806-
}
1807-
1808-
void ValueDecl::setLocalDiscriminator(unsigned index) {
1809-
assert(getDeclContext()->isLocalContext());
1810-
if (!index) {
1811-
assert(!getASTContext().getImpl().LocalDiscriminators.count(this));
1812-
return;
1813-
}
1814-
getASTContext().getImpl().LocalDiscriminators.insert({this, index});
1815-
}
1816-
18171795
NormalProtocolConformance *
18181796
ASTContext::getBehaviorConformance(Type conformingType,
18191797
ProtocolDecl *protocol,
@@ -2034,7 +2012,6 @@ size_t ASTContext::getTotalMemory() const {
20342012
llvm::capacity_in_bytes(getImpl().ModuleLoaders) +
20352013
llvm::capacity_in_bytes(getImpl().RawComments) +
20362014
llvm::capacity_in_bytes(getImpl().BriefComments) +
2037-
llvm::capacity_in_bytes(getImpl().LocalDiscriminators) +
20382015
llvm::capacity_in_bytes(getImpl().ModuleTypes) +
20392016
llvm::capacity_in_bytes(getImpl().GenericParamTypes) +
20402017
// getImpl().GenericFunctionTypes ?

‎lib/AST/Decl.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,6 +1680,16 @@ bool ValueDecl::needsCapture() const {
16801680
return !isa<TypeDecl>(this);
16811681
}
16821682

1683+
unsigned ValueDecl::getLocalDiscriminator() const {
1684+
return LocalDiscriminator;
1685+
}
1686+
1687+
void ValueDecl::setLocalDiscriminator(unsigned index) {
1688+
assert(getDeclContext()->isLocalContext());
1689+
assert(LocalDiscriminator == 0 && "LocalDiscriminator is set multiple times");
1690+
LocalDiscriminator = index;
1691+
}
1692+
16831693
ValueDecl *ValueDecl::getOverriddenDecl() const {
16841694
if (auto fd = dyn_cast<FuncDecl>(this))
16851695
return fd->getOverriddenDecl();

‎lib/IDE/SwiftSourceDocInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ std::pair<bool, Pattern*> NameMatcher::walkToPatternPre(Pattern *P) {
581581
if (isDone() || shouldSkip(P->getSourceRange()))
582582
return std::make_pair(false, P);
583583

584-
tryResolve(ASTWalker::ParentTy(P), P->getLoc());
584+
tryResolve(ASTWalker::ParentTy(P), P->getStartLoc());
585585
return std::make_pair(!isDone(), P);
586586
}
587587

‎lib/Parse/ParseDecl.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2409,6 +2409,14 @@ void Parser::setLocalDiscriminator(ValueDecl *D) {
24092409
D->setLocalDiscriminator(discriminator);
24102410
}
24112411

2412+
void Parser::setLocalDiscriminatorToParamList(ParameterList *PL) {
2413+
for (auto P : *PL) {
2414+
if (!P->hasName() || P->isImplicit())
2415+
continue;
2416+
setLocalDiscriminator(P);
2417+
}
2418+
}
2419+
24122420
void Parser::delayParseFromBeginningToHere(ParserPosition BeginParserPosition,
24132421
ParseDeclOptions Flags) {
24142422
auto CurLoc = Tok.getLoc();
@@ -4282,6 +4290,8 @@ bool Parser::parseGetSetImpl(ParseDeclOptions Flags,
42824290

42834291
// Establish the new context.
42844292
ParseFunctionBody CC(*this, TheDecl);
4293+
for (auto PL : TheDecl->getParameterLists())
4294+
setLocalDiscriminatorToParamList(PL);
42854295

42864296
// Parse the body.
42874297
SmallVector<ASTNode, 16> Entries;
@@ -4373,6 +4383,8 @@ void Parser::parseAccessorBodyDelayed(AbstractFunctionDecl *AFD) {
43734383
// Re-enter the lexical scope.
43744384
Scope S(this, AccessorParserState->takeScope());
43754385
ParseFunctionBody CC(*this, AFD);
4386+
for (auto PL : AFD->getParameterLists())
4387+
setLocalDiscriminatorToParamList(PL);
43764388

43774389
SmallVector<ASTNode, 16> Entries;
43784390
parseBraceItems(Entries);
@@ -4470,8 +4482,6 @@ VarDecl *Parser::parseDeclVarGetSet(Pattern *pattern,
44704482
if (!PrimaryVar || !primaryVarIsWellFormed) {
44714483
diagnose(pattern->getLoc(), diag::getset_nontrivial_pattern);
44724484
Invalid = true;
4473-
} else {
4474-
setLocalDiscriminator(PrimaryVar);
44754485
}
44764486

44774487
TypeLoc TyLoc;
@@ -4881,6 +4891,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
48814891
pattern->forEachVariable([&](VarDecl *VD) {
48824892
VD->setStatic(StaticLoc.isValid());
48834893
VD->getAttrs() = Attributes;
4894+
setLocalDiscriminator(VD);
48844895
Decls.push_back(VD);
48854896
});
48864897

@@ -5326,6 +5337,8 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
53265337

53275338
// Establish the new context.
53285339
ParseFunctionBody CC(*this, FD);
5340+
for (auto PL : FD->getParameterLists())
5341+
setLocalDiscriminatorToParamList(PL);
53295342

53305343
// Check to see if we have a "{" to start a brace statement.
53315344
if (Tok.is(tok::l_brace)) {
@@ -5396,6 +5409,8 @@ bool Parser::parseAbstractFunctionBodyDelayed(AbstractFunctionDecl *AFD) {
53965409
// Re-enter the lexical scope.
53975410
Scope S(this, FunctionParserState->takeScope());
53985411
ParseFunctionBody CC(*this, AFD);
5412+
for (auto PL : AFD->getParameterLists())
5413+
setLocalDiscriminatorToParamList(PL);
53995414

54005415
ParserResult<BraceStmt> Body =
54015416
parseBraceItemList(diag::func_decl_without_brace);
@@ -6258,6 +6273,8 @@ Parser::parseDeclInit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
62586273
} else {
62596274
// Parse the body.
62606275
ParseFunctionBody CC(*this, CD);
6276+
for (auto PL : CD->getParameterLists())
6277+
setLocalDiscriminatorToParamList(PL);
62616278

62626279
if (!isDelayedParsingEnabled()) {
62636280
if (Context.Stats)

‎lib/Parse/ParseExpr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2731,6 +2731,7 @@ ParserResult<Expr> Parser::parseExprClosure() {
27312731
if (params) {
27322732
// Add the parameters into scope.
27332733
addParametersToScope(params);
2734+
setLocalDiscriminatorToParamList(params);
27342735
} else {
27352736
// There are no parameters; allow anonymous closure variables.
27362737
// FIXME: We could do this all the time, and then provide Fix-Its

‎lib/Parse/ParseStmt.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,7 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
975975
// represents tuples and var patterns as tupleexprs and
976976
// unresolved_pattern_expr nodes, instead of as proper pattern nodes.
977977
patternResult.get()->forEachVariable([&](VarDecl *VD) {
978+
P.setLocalDiscriminator(VD);
978979
if (VD->hasName()) P.addToScope(VD);
979980
boundDecls.push_back(VD);
980981
});
@@ -996,6 +997,8 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
996997
for (auto previous : boundDecls) {
997998
if (previous->hasName() && previous->getName() == VD->getName()) {
998999
found = true;
1000+
// Use the same local discriminator.
1001+
VD->setLocalDiscriminator(previous->getLocalDiscriminator());
9991002
break;
10001003
}
10011004
}
@@ -1397,6 +1400,7 @@ Parser::parseStmtConditionElement(SmallVectorImpl<StmtConditionElement> &result,
13971400
// Add variable bindings from the pattern to our current scope and mark
13981401
// them as being having a non-pattern-binding initializer.
13991402
ThePattern.get()->forEachVariable([&](VarDecl *VD) {
1403+
setLocalDiscriminator(VD);
14001404
if (VD->hasName())
14011405
addToScope(VD);
14021406
VD->setHasNonPatternBindingInit();

‎lib/Serialization/ModuleFile.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ class ModuleFile::LocalDeclTableInfo {
429429
public:
430430
using internal_key_type = StringRef;
431431
using external_key_type = internal_key_type;
432-
using data_type = std::pair<DeclID, unsigned>; // ID, local discriminator
432+
using data_type = DeclID;
433433
using hash_value_type = uint32_t;
434434
using offset_type = unsigned;
435435

@@ -447,7 +447,7 @@ class ModuleFile::LocalDeclTableInfo {
447447

448448
static std::pair<unsigned, unsigned> ReadKeyDataLength(const uint8_t *&data) {
449449
unsigned keyLength = endian::readNext<uint16_t, little, unaligned>(data);
450-
return { keyLength, sizeof(uint32_t) + sizeof(unsigned) };
450+
return { keyLength, sizeof(uint32_t) };
451451
}
452452

453453
static internal_key_type ReadKey(const uint8_t *data, unsigned length) {
@@ -456,9 +456,7 @@ class ModuleFile::LocalDeclTableInfo {
456456

457457
static data_type ReadData(internal_key_type key, const uint8_t *data,
458458
unsigned length) {
459-
auto declID = endian::readNext<uint32_t, little, unaligned>(data);
460-
auto discriminator = endian::readNext<unsigned, little, unaligned>(data);
461-
return { declID, discriminator };
459+
return endian::readNext<uint32_t, little, unaligned>(data);
462460
}
463461
};
464462

@@ -1516,7 +1514,7 @@ TypeDecl *ModuleFile::lookupLocalType(StringRef MangledName) {
15161514
if (iter == LocalTypeDecls->end())
15171515
return nullptr;
15181516

1519-
return cast<TypeDecl>(getDecl((*iter).first));
1517+
return cast<TypeDecl>(getDecl(*iter));
15201518
}
15211519

15221520
TypeDecl *ModuleFile::lookupNestedType(Identifier name,
@@ -2021,10 +2019,8 @@ ModuleFile::getLocalTypeDecls(SmallVectorImpl<TypeDecl *> &results) {
20212019
if (!LocalTypeDecls)
20222020
return;
20232021

2024-
for (auto entry : LocalTypeDecls->data()) {
2025-
auto DeclID = entry.first;
2022+
for (auto DeclID : LocalTypeDecls->data()) {
20262023
auto TD = cast<TypeDecl>(getDecl(DeclID));
2027-
TD->setLocalDiscriminator(entry.second);
20282024
results.push_back(TD);
20292025
}
20302026
}

‎lib/Serialization/Serialization.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ namespace {
228228
public:
229229
using key_type = std::string;
230230
using key_type_ref = StringRef;
231-
using data_type = std::pair<DeclID, unsigned>; // ID, local discriminator
231+
using data_type = DeclID;
232232
using data_type_ref = const data_type &;
233233
using hash_value_type = uint32_t;
234234
using offset_type = unsigned;
@@ -242,7 +242,7 @@ namespace {
242242
key_type_ref key,
243243
data_type_ref data) {
244244
uint32_t keyLength = key.size();
245-
uint32_t dataLength = sizeof(uint32_t) + sizeof(unsigned);
245+
uint32_t dataLength = sizeof(uint32_t);
246246
endian::Writer<little> writer(out);
247247
writer.write<uint16_t>(keyLength);
248248
return { keyLength, dataLength };
@@ -256,8 +256,7 @@ namespace {
256256
unsigned len) {
257257
static_assert(declIDFitsIn32Bits(), "DeclID too large");
258258
endian::Writer<little> writer(out);
259-
writer.write<uint32_t>(data.first);
260-
writer.write<unsigned>(data.second);
259+
writer.write<uint32_t>(data);
261260
}
262261
};
263262

@@ -4909,9 +4908,7 @@ void Serializer::writeAST(ModuleOrSourceFile DC,
49094908
Mangle::ASTMangler Mangler;
49104909
std::string MangledName = Mangler.mangleDeclAsUSR(TD, /*USRPrefix*/"");
49114910
assert(!MangledName.empty() && "Mangled type came back empty!");
4912-
localTypeGenerator.insert(MangledName, {
4913-
addDeclRef(TD), TD->getLocalDiscriminator()
4914-
});
4911+
localTypeGenerator.insert(MangledName, addDeclRef(TD));
49154912

49164913
if (auto IDC = dyn_cast<IterableDeclContext>(TD)) {
49174914
collectInterestingNestedDeclarations(*this, IDC->getMembers(),

‎test/refactoring/rename/Outputs/local/casebind_1.swift.expected

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
func test1() {
2+
if true {
3+
let x = 1
4+
print(x)
5+
} else {
6+
let x = 2
7+
print(x)
8+
}
9+
}
10+
11+
func test2(arg1: Int?, arg2: (Int, String)?) {
12+
if let x = arg1 {
13+
print(x)
14+
} else if let (x, y) = arg2 {
15+
print(x, y)
16+
}
17+
}
18+
19+
func test3(arg: Int?) {
20+
switch arg {
21+
case let .some(xRenamed) where xRenamed == 0:
22+
print(xRenamed)
23+
case let .some(x) where x == 1,
24+
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
25+
print(x)
26+
default:
27+
break
28+
}
29+
}
30+
31+
struct Err1 : Error { }
32+
func test4(arg: () throws -> Void) {
33+
do {
34+
try arg()
35+
} catch let x as Err1 {
36+
print(x)
37+
} catch let x {
38+
print(x)
39+
}
40+
}
41+
42+
func test5(_ x: Int) {
43+
let x = x
44+
print(x)
45+
}
46+

‎test/refactoring/rename/Outputs/local/casebind_2.swift.expected

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
func test1() {
2+
if true {
3+
let x = 1
4+
print(x)
5+
} else {
6+
let x = 2
7+
print(x)
8+
}
9+
}
10+
11+
func test2(arg1: Int?, arg2: (Int, String)?) {
12+
if let x = arg1 {
13+
print(x)
14+
} else if let (x, y) = arg2 {
15+
print(x, y)
16+
}
17+
}
18+
19+
func test3(arg: Int?) {
20+
switch arg {
21+
case let .some(x) where x == 0:
22+
print(x)
23+
case let .some(xRenamed) where xRenamed == 1,
24+
let .some(x) where xRenamed == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
25+
print(xRenamed)
26+
default:
27+
break
28+
}
29+
}
30+
31+
struct Err1 : Error { }
32+
func test4(arg: () throws -> Void) {
33+
do {
34+
try arg()
35+
} catch let x as Err1 {
36+
print(x)
37+
} catch let x {
38+
print(x)
39+
}
40+
}
41+
42+
func test5(_ x: Int) {
43+
let x = x
44+
print(x)
45+
}
46+

0 commit comments

Comments
 (0)