[PATCH] D65428: Remove cache for macro arg stringization
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367337: Remove cache for macro arg stringization (authored
by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65428?vs=212261=212384#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65428/new/
https://reviews.llvm.org/D65428
Files:
cfe/trunk/include/clang/Lex/MacroArgs.h
cfe/trunk/lib/Lex/MacroArgs.cpp
cfe/trunk/lib/Lex/TokenLexer.cpp
cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
cfe/trunk/unittests/Lex/LexerTest.cpp
Index: cfe/trunk/include/clang/Lex/MacroArgs.h
===
--- cfe/trunk/include/clang/Lex/MacroArgs.h
+++ cfe/trunk/include/clang/Lex/MacroArgs.h
@@ -48,10 +48,6 @@
/// stream.
std::vector > PreExpArgTokens;
- /// StringifiedArgs - This contains arguments in 'stringified' form. If the
- /// stringified form of an argument has not yet been computed, this is empty.
- std::vector StringifiedArgs;
-
/// ArgCache - This is a linked list of MacroArgs objects that the
/// Preprocessor owns which we use to avoid thrashing malloc/free.
MacroArgs *ArgCache;
@@ -94,12 +90,6 @@
const std::vector &
getPreExpArgument(unsigned Arg, Preprocessor );
- /// getStringifiedArgument - Compute, cache, and return the specified argument
- /// that has been 'stringified' as required by the # operator.
- const Token (unsigned ArgNo, Preprocessor ,
- SourceLocation ExpansionLocStart,
- SourceLocation ExpansionLocEnd);
-
/// getNumMacroArguments - Return the number of arguments the invoked macro
/// expects.
unsigned getNumMacroArguments() const { return NumMacroArgs; }
Index: cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
===
--- cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
+++ cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
+
+// PR39942
+
+class a;
+template a <<(b &, const char *);
+int c;
+#define d(l) l(__FILE__, __LINE__, c)
+#define COMPACT_GOOGLE_LOG_ERROR d(e)
+#define f(g, cond) cond ? (void)0 : h() & g
+#define i(j) COMPACT_GOOGLE_LOG_##j.g()
+#define k(j) f(i(j), 0)
+class e {
+public:
+ e(const char *, int, int);
+ a ();
+};
+class h {
+public:
+ void operator&(a &);
+};
+void use_str(const char *);
+
+#define m(func)\
+ use_str(#func); \
+ k(ERROR) << #func; \
+ return 0; // CHECK: File 1, [[@LINE-1]]:4 -> [[@LINE-1]]:16 = (#0 - #1)
+int main() {
+ m(asdf);
+}
Index: cfe/trunk/lib/Lex/TokenLexer.cpp
===
--- cfe/trunk/lib/Lex/TokenLexer.cpp
+++ cfe/trunk/lib/Lex/TokenLexer.cpp
@@ -383,18 +383,10 @@
SourceLocation ExpansionLocEnd =
getExpansionLocForMacroDefLoc(Tokens[I+1].getLocation());
- Token Res;
- if (CurTok.is(tok::hash)) // Stringify
-Res = ActualArgs->getStringifiedArgument(ArgNo, PP,
- ExpansionLocStart,
- ExpansionLocEnd);
- else {
-// 'charify': don't bother caching these.
-Res = MacroArgs::StringifyArgument(ActualArgs->getUnexpArgument(ArgNo),
- PP, true,
- ExpansionLocStart,
- ExpansionLocEnd);
- }
+ bool Charify = CurTok.is(tok::hashat);
+ const Token *UnexpArg = ActualArgs->getUnexpArgument(ArgNo);
+ Token Res = MacroArgs::StringifyArgument(
+ UnexpArg, PP, Charify, ExpansionLocStart, ExpansionLocEnd);
Res.setFlag(Token::StringifiedInMacro);
// The stringified/charified string leading space flag gets set to match
Index: cfe/trunk/lib/Lex/MacroArgs.cpp
===
--- cfe/trunk/lib/Lex/MacroArgs.cpp
+++ cfe/trunk/lib/Lex/MacroArgs.cpp
@@ -76,8 +76,6 @@
/// destroy - Destroy and deallocate the memory for this object.
///
void MacroArgs::destroy(Preprocessor ) {
- StringifiedArgs.clear();
-
// Don't clear PreExpArgTokens, just clear the entries. Clearing the entries
// would deallocate the element vectors.
for (unsigned i = 0, e = PreExpArgTokens.size(); i != e; ++i)
@@ -307,21 +305,3 @@
ExpansionLocStart, ExpansionLocEnd);
return Tok;
}
-
-///
[PATCH] D65428: Remove cache for macro arg stringization
rnk added a comment. Thanks for looking into it as well, I would not have seen the issue without reviewing the test case again. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65428/new/ https://reviews.llvm.org/D65428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65428: Remove cache for macro arg stringization
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65428/new/ https://reviews.llvm.org/D65428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65428: Remove cache for macro arg stringization
rnk created this revision.
rnk added reviewers: vsk, rsmith.
Herald added a project: clang.
The cache recorded the wrong expansion location for all but the first
stringization. It seems uncommon to stringize the same macro argument
multiple times, so this cache doesn't seem that important.
Fixes PR39942
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65428
Files:
clang/include/clang/Lex/MacroArgs.h
clang/lib/Lex/MacroArgs.cpp
clang/lib/Lex/TokenLexer.cpp
clang/test/CoverageMapping/macro-stringize-twice.cpp
clang/unittests/Lex/LexerTest.cpp
Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -401,18 +401,21 @@
auto MacroArgsDeleter = [](MacroArgs *M) { M->destroy(*PP); };
std::unique_ptr MA(
MacroArgs::create(MI, ArgTokens, false, *PP), MacroArgsDeleter);
- Token Result = MA->getStringifiedArgument(0, *PP, {}, {});
+ auto StringifyArg = [&](int ArgNo) {
+return MA->StringifyArgument(MA->getUnexpArgument(ArgNo), *PP,
+ /*Charify=*/false, {}, {});
+ };
+ Token Result = StringifyArg(0);
EXPECT_EQ(tok::string_literal, Result.getKind());
EXPECT_STREQ("\"\\\"StrArg\\\"\"", Result.getLiteralData());
- Result = MA->getStringifiedArgument(1, *PP, {}, {});
+ Result = StringifyArg(1);
EXPECT_EQ(tok::string_literal, Result.getKind());
EXPECT_STREQ("\"5\"", Result.getLiteralData());
- Result = MA->getStringifiedArgument(2, *PP, {}, {});
+ Result = StringifyArg(2);
EXPECT_EQ(tok::string_literal, Result.getKind());
EXPECT_STREQ("\"'C'\"", Result.getLiteralData());
#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
- EXPECT_DEATH(MA->getStringifiedArgument(3, *PP, {}, {}),
- "Invalid argument number!");
+ EXPECT_DEATH(StringifyArg(3), "Invalid arg #");
#endif
}
Index: clang/test/CoverageMapping/macro-stringize-twice.cpp
===
--- /dev/null
+++ clang/test/CoverageMapping/macro-stringize-twice.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
+
+// PR39942
+
+class a;
+template a <<(b &, const char *);
+int c;
+#define d(l) l(__FILE__, __LINE__, c)
+#define COMPACT_GOOGLE_LOG_ERROR d(e)
+#define f(g, cond) cond ? (void)0 : h() & g
+#define i(j) COMPACT_GOOGLE_LOG_##j.g()
+#define k(j) f(i(j), 0)
+class e {
+public:
+ e(const char *, int, int);
+ a ();
+};
+class h {
+public:
+ void operator&(a &);
+};
+void use_str(const char *);
+
+#define m(func)\
+ use_str(#func); \
+ k(ERROR) << #func; \
+ return 0; // CHECK: File 1, [[@LINE-1]]:4 -> [[@LINE-1]]:16 = (#0 - #1)
+int main() {
+ m(asdf);
+}
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -383,18 +383,10 @@
SourceLocation ExpansionLocEnd =
getExpansionLocForMacroDefLoc(Tokens[I+1].getLocation());
- Token Res;
- if (CurTok.is(tok::hash)) // Stringify
-Res = ActualArgs->getStringifiedArgument(ArgNo, PP,
- ExpansionLocStart,
- ExpansionLocEnd);
- else {
-// 'charify': don't bother caching these.
-Res = MacroArgs::StringifyArgument(ActualArgs->getUnexpArgument(ArgNo),
- PP, true,
- ExpansionLocStart,
- ExpansionLocEnd);
- }
+ bool Charify = CurTok.is(tok::hashat);
+ const Token *UnexpArg = ActualArgs->getUnexpArgument(ArgNo);
+ Token Res = MacroArgs::StringifyArgument(
+ UnexpArg, PP, Charify, ExpansionLocStart, ExpansionLocEnd);
Res.setFlag(Token::StringifiedInMacro);
// The stringified/charified string leading space flag gets set to match
Index: clang/lib/Lex/MacroArgs.cpp
===
--- clang/lib/Lex/MacroArgs.cpp
+++ clang/lib/Lex/MacroArgs.cpp
@@ -76,8 +76,6 @@
/// destroy - Destroy and deallocate the memory for this object.
///
void MacroArgs::destroy(Preprocessor ) {
- StringifiedArgs.clear();
-
// Don't clear PreExpArgTokens, just clear the entries. Clearing the entries
// would deallocate the element vectors.
for (unsigned i = 0, e = PreExpArgTokens.size(); i != e; ++i)
@@ -307,21 +305,3 @@
ExpansionLocStart, ExpansionLocEnd);
return Tok;
}
-
-/// getStringifiedArgument - Compute, cache, and return the specified
