Skip to content

Commit dfed20c

Browse files
rintarorintaro
authored
Merge pull request #16536 from rintaro/4.2-parse-var-discriminator
[4.2] Discriminate local variables
2 parents 4861b0b + fd1e14f commit dfed20c

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
@@ -763,6 +763,7 @@ class Parser {
763763
ParserStatus parseLineDirective(bool isLine = false);
764764

765765
void setLocalDiscriminator(ValueDecl *D);
766+
void setLocalDiscriminatorToParamList(ParameterList *PL);
766767

767768
/// Parse the optional attributes before a declaration.
768769
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 = 412; // Last change: add begin_access [builtin].
58+
const uint16_t VERSION_MINOR = 413; // 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
@@ -223,10 +223,6 @@ FOR_KNOWN_FOUNDATION_TYPES(CACHE_FOUNDATION_DECL)
223223
/// \brief Map from Swift declarations to brief comments.
224224
llvm::DenseMap<const Decl *, StringRef> BriefComments;
225225

226-
/// \brief Map from local declarations to their discriminators.
227-
/// Missing entries implicitly have value 0.
228-
llvm::DenseMap<const ValueDecl *, unsigned> LocalDiscriminators;
229-
230226
/// \brief Map from declarations to foreign error conventions.
231227
/// This applies to both actual imported functions and to @objc functions.
232228
llvm::DenseMap<const AbstractFunctionDecl *,
@@ -1769,24 +1765,6 @@ void ASTContext::setBriefComment(const Decl *D, StringRef Comment) {
17691765
Impl.BriefComments[D] = Comment;
17701766
}
17711767

1772-
unsigned ValueDecl::getLocalDiscriminator() const {
1773-
assert(getDeclContext()->isLocalContext());
1774-
auto &discriminators = getASTContext().Impl.LocalDiscriminators;
1775-
auto it = discriminators.find(this);
1776-
if (it == discriminators.end())
1777-
return 0;
1778-
return it->second;
1779-
}
1780-
1781-
void ValueDecl::setLocalDiscriminator(unsigned index) {
1782-
assert(getDeclContext()->isLocalContext());
1783-
if (!index) {
1784-
assert(!getASTContext().Impl.LocalDiscriminators.count(this));
1785-
return;
1786-
}
1787-
getASTContext().Impl.LocalDiscriminators.insert({this, index});
1788-
}
1789-
17901768
NormalProtocolConformance *
17911769
ASTContext::getBehaviorConformance(Type conformingType,
17921770
ProtocolDecl *protocol,
@@ -2031,7 +2009,6 @@ size_t ASTContext::getTotalMemory() const {
20312009
llvm::capacity_in_bytes(Impl.ModuleLoaders) +
20322010
llvm::capacity_in_bytes(Impl.RawComments) +
20332011
llvm::capacity_in_bytes(Impl.BriefComments) +
2034-
llvm::capacity_in_bytes(Impl.LocalDiscriminators) +
20352012
llvm::capacity_in_bytes(Impl.ModuleTypes) +
20362013
llvm::capacity_in_bytes(Impl.GenericParamTypes) +
20372014
// Impl.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
@@ -2341,6 +2341,14 @@ void Parser::setLocalDiscriminator(ValueDecl *D) {
23412341
D->setLocalDiscriminator(discriminator);
23422342
}
23432343

2344+
void Parser::setLocalDiscriminatorToParamList(ParameterList *PL) {
2345+
for (auto P : *PL) {
2346+
if (!P->hasName() || P->isImplicit())
2347+
continue;
2348+
setLocalDiscriminator(P);
2349+
}
2350+
}
2351+
23442352
void Parser::delayParseFromBeginningToHere(ParserPosition BeginParserPosition,
23452353
ParseDeclOptions Flags) {
23462354
auto CurLoc = Tok.getLoc();
@@ -4196,6 +4204,8 @@ bool Parser::parseGetSetImpl(ParseDeclOptions Flags,
41964204

41974205
// Establish the new context.
41984206
ParseFunctionBody CC(*this, TheDecl);
4207+
for (auto PL : TheDecl->getParameterLists())
4208+
setLocalDiscriminatorToParamList(PL);
41994209

42004210
// Parse the body.
42014211
SmallVector<ASTNode, 16> Entries;
@@ -4287,6 +4297,8 @@ void Parser::parseAccessorBodyDelayed(AbstractFunctionDecl *AFD) {
42874297
// Re-enter the lexical scope.
42884298
Scope S(this, AccessorParserState->takeScope());
42894299
ParseFunctionBody CC(*this, AFD);
4300+
for (auto PL : AFD->getParameterLists())
4301+
setLocalDiscriminatorToParamList(PL);
42904302

42914303
SmallVector<ASTNode, 16> Entries;
42924304
parseBraceItems(Entries);
@@ -4384,8 +4396,6 @@ VarDecl *Parser::parseDeclVarGetSet(Pattern *pattern,
43844396
if (!PrimaryVar || !primaryVarIsWellFormed) {
43854397
diagnose(pattern->getLoc(), diag::getset_nontrivial_pattern);
43864398
Invalid = true;
4387-
} else {
4388-
setLocalDiscriminator(PrimaryVar);
43894399
}
43904400

43914401
TypeLoc TyLoc;
@@ -4795,6 +4805,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
47954805
pattern->forEachVariable([&](VarDecl *VD) {
47964806
VD->setStatic(StaticLoc.isValid());
47974807
VD->getAttrs() = Attributes;
4808+
setLocalDiscriminator(VD);
47984809
Decls.push_back(VD);
47994810
});
48004811

@@ -5240,6 +5251,8 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
52405251

52415252
// Establish the new context.
52425253
ParseFunctionBody CC(*this, FD);
5254+
for (auto PL : FD->getParameterLists())
5255+
setLocalDiscriminatorToParamList(PL);
52435256

52445257
// Check to see if we have a "{" to start a brace statement.
52455258
if (Tok.is(tok::l_brace)) {
@@ -5310,6 +5323,8 @@ bool Parser::parseAbstractFunctionBodyDelayed(AbstractFunctionDecl *AFD) {
53105323
// Re-enter the lexical scope.
53115324
Scope S(this, FunctionParserState->takeScope());
53125325
ParseFunctionBody CC(*this, AFD);
5326+
for (auto PL : AFD->getParameterLists())
5327+
setLocalDiscriminatorToParamList(PL);
53135328

53145329
ParserResult<BraceStmt> Body =
53155330
parseBraceItemList(diag::func_decl_without_brace);
@@ -6149,6 +6164,8 @@ Parser::parseDeclInit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
61496164
} else {
61506165
// Parse the body.
61516166
ParseFunctionBody CC(*this, CD);
6167+
for (auto PL : CD->getParameterLists())
6168+
setLocalDiscriminatorToParamList(PL);
61526169

61536170
if (!isDelayedParsingEnabled()) {
61546171
if (Context.Stats)

‎lib/Parse/ParseExpr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2727,6 +2727,7 @@ ParserResult<Expr> Parser::parseExprClosure() {
27272727
if (params) {
27282728
// Add the parameters into scope.
27292729
addParametersToScope(params);
2730+
setLocalDiscriminatorToParamList(params);
27302731
} else {
27312732
// There are no parameters; allow anonymous closure variables.
27322733
// 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
@@ -972,6 +972,7 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
972972
// represents tuples and var patterns as tupleexprs and
973973
// unresolved_pattern_expr nodes, instead of as proper pattern nodes.
974974
patternResult.get()->forEachVariable([&](VarDecl *VD) {
975+
P.setLocalDiscriminator(VD);
975976
if (VD->hasName()) P.addToScope(VD);
976977
boundDecls.push_back(VD);
977978
});
@@ -993,6 +994,8 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
993994
for (auto previous : boundDecls) {
994995
if (previous->hasName() && previous->getName() == VD->getName()) {
995996
found = true;
997+
// Use the same local discriminator.
998+
VD->setLocalDiscriminator(previous->getLocalDiscriminator());
996999
break;
9971000
}
9981001
}
@@ -1390,6 +1393,7 @@ Parser::parseStmtConditionElement(SmallVectorImpl<StmtConditionElement> &result,
13901393
// Add variable bindings from the pattern to our current scope and mark
13911394
// them as being having a non-pattern-binding initializer.
13921395
ThePattern.get()->forEachVariable([&](VarDecl *VD) {
1396+
setLocalDiscriminator(VD);
13931397
if (VD->hasName())
13941398
addToScope(VD);
13951399
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

@@ -1527,7 +1525,7 @@ TypeDecl *ModuleFile::lookupLocalType(StringRef MangledName) {
15271525
if (iter == LocalTypeDecls->end())
15281526
return nullptr;
15291527

1530-
return cast<TypeDecl>(getDecl((*iter).first));
1528+
return cast<TypeDecl>(getDecl(*iter));
15311529
}
15321530

15331531
TypeDecl *ModuleFile::lookupNestedType(Identifier name,
@@ -2015,10 +2013,8 @@ ModuleFile::getLocalTypeDecls(SmallVectorImpl<TypeDecl *> &results) {
20152013
if (!LocalTypeDecls)
20162014
return;
20172015

2018-
for (auto entry : LocalTypeDecls->data()) {
2019-
auto DeclID = entry.first;
2016+
for (auto DeclID : LocalTypeDecls->data()) {
20202017
auto TD = cast<TypeDecl>(getDecl(DeclID));
2021-
TD->setLocalDiscriminator(entry.second);
20222018
results.push_back(TD);
20232019
}
20242020
}

‎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

@@ -4913,9 +4912,7 @@ void Serializer::writeAST(ModuleOrSourceFile DC,
49134912
Mangle::ASTMangler Mangler;
49144913
std::string MangledName = Mangler.mangleDeclAsUSR(TD, /*USRPrefix*/"");
49154914
assert(!MangledName.empty() && "Mangled type came back empty!");
4916-
localTypeGenerator.insert(MangledName, {
4917-
addDeclRef(TD), TD->getLocalDiscriminator()
4918-
});
4915+
localTypeGenerator.insert(MangledName, addDeclRef(TD));
49194916

49204917
if (auto IDC = dyn_cast<IterableDeclContext>(TD)) {
49214918
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)