Skip to content

Commit 36b949c

Browse files
beccadaxbeccadax
committed
[NFC] Improve identifier usage in AST transforms
Change the various AST transforms to use prebuilt DeclName constants more heavily rather than an ad-hoc mix of Identifiers, string literals, std::strings, and StringRefs with questionable memory management.
1 parent 4afffbe commit 36b949c

File tree

5 files changed

+70
-61
lines changed

5 files changed

+70
-61
lines changed

‎lib/Sema/DebuggerTestingTransform.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,14 @@ class DebuggerTestingTransform : public ASTWalker {
6767
ASTContext &Ctx;
6868
DiscriminatorFinder &DF;
6969
std::vector<DeclContext *> LocalDeclContextStack;
70+
const DeclName StringForPrintObjectName;
71+
const DeclName DebuggerTestingCheckExpectName;
7072

7173
public:
7274
DebuggerTestingTransform(ASTContext &Ctx, DiscriminatorFinder &DF)
73-
: Ctx(Ctx), DF(DF) {}
75+
: Ctx(Ctx), DF(DF),
76+
StringForPrintObjectName((Ctx.getIdentifier("_stringForPrintObject"))),
77+
DebuggerTestingCheckExpectName((Ctx.getIdentifier("_debuggerTestingCheckExpect"))) {}
7478

7579
bool walkToDeclPre(Decl *D) override {
7680
pushLocalDeclContext(D);
@@ -200,7 +204,7 @@ class DebuggerTestingTransform : public ASTWalker {
200204

201205
// Create _stringForPrintObject($Varname).
202206
auto *PODeclRef = new (Ctx)
203-
UnresolvedDeclRefExpr(Ctx.getIdentifier("_stringForPrintObject"),
207+
UnresolvedDeclRefExpr(StringForPrintObjectName,
204208
DeclRefKind::Ordinary, DeclNameLoc());
205209
Expr *POArgs[] = {DstRef};
206210
Identifier POLabels[] = {Identifier()};
@@ -211,7 +215,7 @@ class DebuggerTestingTransform : public ASTWalker {
211215
Identifier CheckExpectLabels[] = {Identifier(), Identifier()};
212216
Expr *CheckExpectArgs[] = {Varname, POCall};
213217
UnresolvedDeclRefExpr *CheckExpectDRE = new (Ctx)
214-
UnresolvedDeclRefExpr(Ctx.getIdentifier("_debuggerTestingCheckExpect"),
218+
UnresolvedDeclRefExpr(DebuggerTestingCheckExpectName,
215219
DeclRefKind::Ordinary, DeclNameLoc());
216220
auto *CheckExpectExpr = CallExpr::createImplicit(
217221
Ctx, CheckExpectDRE, CheckExpectArgs, CheckExpectLabels);

‎lib/Sema/InstrumenterSupport.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ InstrumenterBase::InstrumenterBase(ASTContext &C, DeclContext *DC)
9090
TypeCheckDC->getParentModule()->lookupValue(
9191
moduleIdentifier, NLKind::UnqualifiedLookup, results);
9292

93-
ModuleIdentifier = (results.size() == 1) ? moduleIdentifier : Identifier();
93+
if (results.size() == 1)
94+
ModuleIdentifier = results.front()->getFullName();
9495

9596
// Setup File identifier
9697
StringRef filePath = TypeCheckDC->getParentSourceFile()->getFilename();
@@ -106,7 +107,8 @@ InstrumenterBase::InstrumenterBase(ASTContext &C, DeclContext *DC)
106107
TypeCheckDC->getParentModule()->lookupValue(
107108
fileIdentifier, NLKind::UnqualifiedLookup, results);
108109

109-
FileIdentifier = (results.size() == 1) ? fileIdentifier : Identifier();
110+
if (results.size() == 1)
111+
FileIdentifier = results.front()->getFullName();
110112
}
111113

112114
void InstrumenterBase::anchor() {}
@@ -128,3 +130,13 @@ bool InstrumenterBase::doTypeCheckImpl(ASTContext &Ctx, DeclContext *DC,
128130

129131
return false;
130132
}
133+
134+
Expr *InstrumenterBase::buildIDArgumentExpr(Optional<DeclName> name,
135+
SourceRange SR) {
136+
if (!name)
137+
return IntegerLiteralExpr::createFromUnsigned(Context, 0);
138+
139+
return new (Context) UnresolvedDeclRefExpr(*name, DeclRefKind::Ordinary,
140+
DeclNameLoc(SR.End));
141+
}
142+

‎lib/Sema/InstrumenterSupport.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,19 @@ class InstrumenterBase {
4242
protected:
4343
ASTContext &Context;
4444
DeclContext *TypeCheckDC;
45-
Identifier ModuleIdentifier;
46-
Identifier FileIdentifier;
45+
Optional<DeclName> ModuleIdentifier;
46+
Optional<DeclName> FileIdentifier;
4747

4848
InstrumenterBase(ASTContext &C, DeclContext *DC);
4949
virtual ~InstrumenterBase() = default;
5050
virtual void anchor();
5151
virtual BraceStmt *transformBraceStmt(BraceStmt *BS,
5252
bool TopLevel = false) = 0;
5353

54+
/// Create an expression which retrieves a valid ModuleIdentifier or
55+
/// FileIdentifier, if available.
56+
Expr *buildIDArgumentExpr(Optional<DeclName> name, SourceRange SR);
57+
5458
class ClosureFinder : public ASTWalker {
5559
private:
5660
InstrumenterBase &I;

‎lib/Sema/PCMacro.cpp

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,14 @@ namespace {
4040
class Instrumenter : InstrumenterBase {
4141
private:
4242
unsigned &TmpNameIndex;
43+
DeclName LogBeforeName;
44+
DeclName LogAfterName;
4345

4446
public:
4547
Instrumenter(ASTContext &C, DeclContext *DC, unsigned &TmpNameIndex)
46-
: InstrumenterBase(C, DC), TmpNameIndex(TmpNameIndex) {}
48+
: InstrumenterBase(C, DC), TmpNameIndex(TmpNameIndex),
49+
LogBeforeName((C.getIdentifier("__builtin_pc_before"))),
50+
LogAfterName((C.getIdentifier("__builtin_pc_after"))) {}
4751

4852
Stmt *transformStmt(Stmt *S) {
4953
switch (S->getKind()) {
@@ -494,9 +498,9 @@ class Instrumenter : InstrumenterBase {
494498

495499
Added<Stmt *> buildLoggerCall(SourceRange SR, bool isBefore) {
496500
if (isBefore) {
497-
return buildLoggerCallWithArgs("__builtin_pc_before", SR);
501+
return buildLoggerCallWithArgs(LogBeforeName, SR);
498502
} else {
499-
return buildLoggerCallWithArgs("__builtin_pc_after", SR);
503+
return buildLoggerCallWithArgs(LogAfterName, SR);
500504
}
501505
}
502506

@@ -534,25 +538,16 @@ class Instrumenter : InstrumenterBase {
534538
Expr *StartColumn = IntegerLiteralExpr::createFromUnsigned(Context, StartLC.second);
535539
Expr *EndColumn = IntegerLiteralExpr::createFromUnsigned(Context, EndLC.second);
536540

537-
Expr *ModuleExpr =
538-
!ModuleIdentifier.empty()
539-
? (Expr *)new (Context) UnresolvedDeclRefExpr(
540-
ModuleIdentifier, DeclRefKind::Ordinary, DeclNameLoc(SR.End))
541-
: (Expr *)IntegerLiteralExpr::createFromUnsigned(Context, 0);
542-
543-
Expr *FileExpr =
544-
!FileIdentifier.empty()
545-
? (Expr *)new (Context) UnresolvedDeclRefExpr(
546-
FileIdentifier, DeclRefKind::Ordinary, DeclNameLoc(SR.End))
547-
: (Expr *)IntegerLiteralExpr::createFromUnsigned(Context, 0);
541+
Expr *ModuleExpr = buildIDArgumentExpr(ModuleIdentifier, SR);
542+
Expr *FileExpr = buildIDArgumentExpr(FileIdentifier, SR);
548543

549544
llvm::SmallVector<Expr *, 6> ArgsWithSourceRange{};
550545

551546
ArgsWithSourceRange.append(
552547
{StartLine, EndLine, StartColumn, EndColumn, ModuleExpr, FileExpr});
553548

554549
UnresolvedDeclRefExpr *BeforeLoggerRef = new (Context)
555-
UnresolvedDeclRefExpr(Context.getIdentifier("__builtin_pc_before"),
550+
UnresolvedDeclRefExpr(LogBeforeName,
556551
DeclRefKind::Ordinary, DeclNameLoc(SR.End));
557552
BeforeLoggerRef->setImplicit(true);
558553
SmallVector<Identifier, 6> ArgLabels(ArgsWithSourceRange.size(),
@@ -566,7 +561,7 @@ class Instrumenter : InstrumenterBase {
566561
}
567562

568563
UnresolvedDeclRefExpr *AfterLoggerRef = new (Context)
569-
UnresolvedDeclRefExpr(Context.getIdentifier("__builtin_pc_after"),
564+
UnresolvedDeclRefExpr(LogAfterName,
570565
DeclRefKind::Ordinary, DeclNameLoc(SR.End));
571566
AfterLoggerRef->setImplicit(true);
572567
ApplyExpr *AfterLoggerCall = CallExpr::createImplicit(
@@ -597,7 +592,7 @@ class Instrumenter : InstrumenterBase {
597592
return *AddedGet;
598593
}
599594

600-
Added<Stmt *> buildLoggerCallWithArgs(const char *LoggerName,
595+
Added<Stmt *> buildLoggerCallWithArgs(DeclName LoggerName,
601596
SourceRange SR) {
602597
if (!SR.isValid()) {
603598
return nullptr;
@@ -614,25 +609,16 @@ class Instrumenter : InstrumenterBase {
614609
Expr *StartColumn = IntegerLiteralExpr::createFromUnsigned(Context, StartLC.second);
615610
Expr *EndColumn = IntegerLiteralExpr::createFromUnsigned(Context, EndLC.second);
616611

617-
Expr *ModuleExpr =
618-
!ModuleIdentifier.empty()
619-
? (Expr *)new (Context) UnresolvedDeclRefExpr(
620-
ModuleIdentifier, DeclRefKind::Ordinary, DeclNameLoc(SR.End))
621-
: (Expr *)IntegerLiteralExpr::createFromUnsigned(Context, 0);
622-
623-
Expr *FileExpr =
624-
!FileIdentifier.empty()
625-
? (Expr *)new (Context) UnresolvedDeclRefExpr(
626-
FileIdentifier, DeclRefKind::Ordinary, DeclNameLoc(SR.End))
627-
: (Expr *)IntegerLiteralExpr::createFromUnsigned(Context, 0);
612+
Expr *ModuleExpr = buildIDArgumentExpr(ModuleIdentifier, SR);
613+
Expr *FileExpr = buildIDArgumentExpr(FileIdentifier, SR);
628614

629615
llvm::SmallVector<Expr *, 6> ArgsWithSourceRange{};
630616

631617
ArgsWithSourceRange.append(
632618
{StartLine, EndLine, StartColumn, EndColumn, ModuleExpr, FileExpr});
633619

634620
UnresolvedDeclRefExpr *LoggerRef = new (Context)
635-
UnresolvedDeclRefExpr(Context.getIdentifier(LoggerName),
621+
UnresolvedDeclRefExpr(LoggerName,
636622
DeclRefKind::Ordinary, DeclNameLoc(SR.End));
637623

638624
LoggerRef->setImplicit(true);

‎lib/Sema/PlaygroundTransform.cpp

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ class Instrumenter : InstrumenterBase {
4242
unsigned &TmpNameIndex;
4343
bool HighPerformance;
4444

45+
DeclName DebugPrintName;
46+
DeclName PrintName;
47+
DeclName PostPrintName;
48+
DeclName LogWithIDName;
49+
DeclName LogScopeExitName;
50+
DeclName LogScopeEntryName;
51+
DeclName SendDataName;
52+
4553
struct BracePair {
4654
public:
4755
SourceRange BraceRange;
@@ -117,7 +125,14 @@ class Instrumenter : InstrumenterBase {
117125
Instrumenter(ASTContext &C, DeclContext *DC, std::mt19937_64 &RNG, bool HP,
118126
unsigned &TmpNameIndex)
119127
: InstrumenterBase(C, DC), RNG(RNG), TmpNameIndex(TmpNameIndex),
120-
HighPerformance(HP) {}
128+
HighPerformance(HP),
129+
DebugPrintName((C.getIdentifier("__builtin_debugPrint"))),
130+
PrintName((C.getIdentifier("__builtin_print"))),
131+
PostPrintName((C.getIdentifier("__builtin_postPrint"))),
132+
LogWithIDName((C.getIdentifier("__builtin_log_with_id"))),
133+
LogScopeExitName((C.getIdentifier("__builtin_log_scope_exit"))),
134+
LogScopeEntryName((C.getIdentifier("__builtin_log_scope_entry"))),
135+
SendDataName((C.getIdentifier("__builtin_send_data"))) { }
121136

122137
Stmt *transformStmt(Stmt *S) {
123138
switch (S->getKind()) {
@@ -699,11 +714,10 @@ class Instrumenter : InstrumenterBase {
699714
Added<Stmt *> logPrint(bool isDebugPrint, ApplyExpr *AE,
700715
PatternBindingDecl *&ArgPattern,
701716
VarDecl *&ArgVariable) {
702-
const char *LoggerName =
703-
isDebugPrint ? "__builtin_debugPrint" : "__builtin_print";
717+
DeclName LoggerName = isDebugPrint ? DebugPrintName : PrintName;
704718

705719
UnresolvedDeclRefExpr *LoggerRef = new (Context) UnresolvedDeclRefExpr(
706-
Context.getIdentifier(LoggerName), DeclRefKind::Ordinary,
720+
LoggerName, DeclRefKind::Ordinary,
707721
DeclNameLoc(AE->getSourceRange().End));
708722

709723
std::tie(ArgPattern, ArgVariable) = maybeFixupPrintArgument(AE);
@@ -719,8 +733,7 @@ class Instrumenter : InstrumenterBase {
719733
}
720734

721735
Added<Stmt *> logPostPrint(SourceRange SR) {
722-
return buildLoggerCallWithArgs("__builtin_postPrint",
723-
MutableArrayRef<Expr *>(), SR);
736+
return buildLoggerCallWithArgs(PostPrintName, {}, SR);
724737
}
725738

726739
std::pair<PatternBindingDecl *, VarDecl *>
@@ -770,7 +783,7 @@ class Instrumenter : InstrumenterBase {
770783

771784
Expr *LoggerArgExprs[] = {*E, NameExpr, IDExpr};
772785

773-
return buildLoggerCallWithArgs("__builtin_log_with_id",
786+
return buildLoggerCallWithArgs(LogWithIDName,
774787
MutableArrayRef<Expr *>(LoggerArgExprs), SR);
775788
}
776789

@@ -783,13 +796,12 @@ class Instrumenter : InstrumenterBase {
783796
}
784797

785798
Added<Stmt *> buildScopeCall(SourceRange SR, bool IsExit) {
786-
const char *LoggerName =
787-
IsExit ? "__builtin_log_scope_exit" : "__builtin_log_scope_entry";
799+
auto LoggerName = IsExit ? LogScopeExitName : LogScopeEntryName;
788800

789801
return buildLoggerCallWithArgs(LoggerName, MutableArrayRef<Expr *>(), SR);
790802
}
791803

792-
Added<Stmt *> buildLoggerCallWithArgs(const char *LoggerName,
804+
Added<Stmt *> buildLoggerCallWithArgs(DeclName LoggerName,
793805
MutableArrayRef<Expr *> Args,
794806
SourceRange SR) {
795807
// If something doesn't have a valid source range it can not be playground
@@ -809,26 +821,17 @@ class Instrumenter : InstrumenterBase {
809821
Expr *StartColumn = IntegerLiteralExpr::createFromUnsigned(Context, StartLC.second);
810822
Expr *EndColumn = IntegerLiteralExpr::createFromUnsigned(Context, EndLC.second);
811823

812-
Expr *ModuleExpr =
813-
!ModuleIdentifier.empty()
814-
? (Expr *)new (Context) UnresolvedDeclRefExpr(
815-
ModuleIdentifier, DeclRefKind::Ordinary, DeclNameLoc(SR.End))
816-
: (Expr *)IntegerLiteralExpr::createFromUnsigned(Context, 0);
817-
818-
Expr *FileExpr =
819-
!FileIdentifier.empty()
820-
? (Expr *)new (Context) UnresolvedDeclRefExpr(
821-
FileIdentifier, DeclRefKind::Ordinary, DeclNameLoc(SR.End))
822-
: (Expr *)IntegerLiteralExpr::createFromUnsigned(Context, 0);
824+
Expr *ModuleExpr = buildIDArgumentExpr(ModuleIdentifier, SR);
825+
Expr *FileExpr = buildIDArgumentExpr(FileIdentifier, SR);
823826

824827
llvm::SmallVector<Expr *, 6> ArgsWithSourceRange(Args.begin(), Args.end());
825828

826829
ArgsWithSourceRange.append(
827830
{StartLine, EndLine, StartColumn, EndColumn, ModuleExpr, FileExpr});
828831

829832
UnresolvedDeclRefExpr *LoggerRef = new (Context)
830-
UnresolvedDeclRefExpr(Context.getIdentifier(LoggerName),
831-
DeclRefKind::Ordinary, DeclNameLoc(SR.End));
833+
UnresolvedDeclRefExpr(LoggerName, DeclRefKind::Ordinary,
834+
DeclNameLoc(SR.End));
832835

833836
LoggerRef->setImplicit(true);
834837

@@ -857,8 +860,8 @@ class Instrumenter : InstrumenterBase {
857860
AccessSemantics::Ordinary, Apply->getType());
858861

859862
UnresolvedDeclRefExpr *SendDataRef = new (Context)
860-
UnresolvedDeclRefExpr(Context.getIdentifier("__builtin_send_data"),
861-
DeclRefKind::Ordinary, DeclNameLoc());
863+
UnresolvedDeclRefExpr(SendDataName, DeclRefKind::Ordinary,
864+
DeclNameLoc());
862865

863866
SendDataRef->setImplicit(true);
864867

0 commit comments

Comments
 (0)