From c0e275df3d5df8b8f400322d12d011f11eba39b9 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Tue, 30 Jul 2019 17:58:22 +0000 Subject: [PATCH] Remove cache for macro arg stringization Summary: 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 Reviewers: vsk, rsmith Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D65428 llvm-svn: 367337 --- clang/include/clang/Lex/MacroArgs.h | 10 -------- clang/lib/Lex/MacroArgs.cpp | 20 --------------- clang/lib/Lex/TokenLexer.cpp | 16 +++--------- .../test/CoverageMapping/macro-stringize-twice.cpp | 30 ++++++++++++++++++++++ clang/unittests/Lex/LexerTest.cpp | 13 ++++++---- 5 files changed, 42 insertions(+), 47 deletions(-) create mode 100644 clang/test/CoverageMapping/macro-stringize-twice.cpp diff --git a/clang/include/clang/Lex/MacroArgs.h b/clang/include/clang/Lex/MacroArgs.h index 8806f2d..59676c3 100644 --- a/clang/include/clang/Lex/MacroArgs.h +++ b/clang/include/clang/Lex/MacroArgs.h @@ -48,10 +48,6 @@ class MacroArgs final /// 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 @@ public: const std::vector & getPreExpArgument(unsigned Arg, Preprocessor &PP); - /// getStringifiedArgument - Compute, cache, and return the specified argument - /// that has been 'stringified' as required by the # operator. - const Token &getStringifiedArgument(unsigned ArgNo, Preprocessor &PP, - SourceLocation ExpansionLocStart, - SourceLocation ExpansionLocEnd); - /// getNumMacroArguments - Return the number of arguments the invoked macro /// expects. unsigned getNumMacroArguments() const { return NumMacroArgs; } diff --git a/clang/lib/Lex/MacroArgs.cpp b/clang/lib/Lex/MacroArgs.cpp index 5aa4679..7ede00b 100644 --- a/clang/lib/Lex/MacroArgs.cpp +++ b/clang/lib/Lex/MacroArgs.cpp @@ -76,8 +76,6 @@ MacroArgs *MacroArgs::create(const MacroInfo *MI, /// destroy - Destroy and deallocate the memory for this object. /// void MacroArgs::destroy(Preprocessor &PP) { - 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 @@ Token MacroArgs::StringifyArgument(const Token *ArgToks, ExpansionLocStart, ExpansionLocEnd); return Tok; } - -/// getStringifiedArgument - Compute, cache, and return the specified argument -/// that has been 'stringified' as required by the # operator. -const Token &MacroArgs::getStringifiedArgument(unsigned ArgNo, - Preprocessor &PP, - SourceLocation ExpansionLocStart, - SourceLocation ExpansionLocEnd) { - assert(ArgNo < getNumMacroArguments() && "Invalid argument number!"); - if (StringifiedArgs.empty()) - StringifiedArgs.resize(getNumMacroArguments(), {}); - - if (StringifiedArgs[ArgNo].isNot(tok::string_literal)) - StringifiedArgs[ArgNo] = StringifyArgument(getUnexpArgument(ArgNo), PP, - /*Charify=*/false, - ExpansionLocStart, - ExpansionLocEnd); - return StringifiedArgs[ArgNo]; -} diff --git a/clang/lib/Lex/TokenLexer.cpp b/clang/lib/Lex/TokenLexer.cpp index a7957e8..da5681a 100644 --- a/clang/lib/Lex/TokenLexer.cpp +++ b/clang/lib/Lex/TokenLexer.cpp @@ -383,18 +383,10 @@ void TokenLexer::ExpandFunctionArguments() { 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 diff --git a/clang/test/CoverageMapping/macro-stringize-twice.cpp b/clang/test/CoverageMapping/macro-stringize-twice.cpp new file mode 100644 index 0000000..7a91d91 --- /dev/null +++ b/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 &operator<<(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 &g(); +}; +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); +} diff --git a/clang/unittests/Lex/LexerTest.cpp b/clang/unittests/Lex/LexerTest.cpp index 7b14f56..29eea42 100644 --- a/clang/unittests/Lex/LexerTest.cpp +++ b/clang/unittests/Lex/LexerTest.cpp @@ -401,18 +401,21 @@ TEST_F(LexerTest, DontOverallocateStringifyArgs) { auto MacroArgsDeleter = [&PP](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 } -- 2.7.4