Remove cache for macro arg stringization

[rnk]

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

[vsk]

Thanks, lgtm.

[rnk]

Thanks for looking into it as well, I would not have seen the issue without reviewing the test case again. :)

cfe/trunk/include/clang/Lex/MacroArgs.h

class MacroArgs final
   /// is false.
   bool VarargsElided;
 
   /// PreExpArgTokens - Pre-expanded tokens for arguments that need them.  Empty
   /// if not yet computed.  This includes the EOF marker at the end of the
   /// stream.
   std::vector<std::vector<Token> > 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<Token> StringifiedArgs;
-
   /// ArgCache - This is a linked list of MacroArgs objects that the
   /// Preprocessor owns which we use to avoid thrashing malloc/free.
   MacroArgs *ArgCache;
 
   /// MacroArgs - The number of arguments the invoked macro expects.
   unsigned NumMacroArgs;
 
   MacroArgs(unsigned NumToks, bool varargsElided, unsigned MacroArgs)
public:
   /// argument.
   static unsigned getArgLength(const Token *ArgPtr);
 
   /// getPreExpArgument - Return the pre-expanded form of the specified
   /// argument.
   const std::vector<Token> &
     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; }
 
   /// isVarargsElidedUse - Return true if this is a C99 style varargs macro
   /// invocation and there was no argument specified for the "..." argument.  If
   /// the argument was specified (even empty) or this isn't a C99 style varargs
   /// function, or if in strict mode and the C99 varargs macro had only a ...

cfe/trunk/lib/Lex/MacroArgs.cpp

MacroArgs *MacroArgs::create(const MacroInfo *MI,
   }
 
   return Result;
 }
 
 /// 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)
     PreExpArgTokens[i].clear();
 
   // Add this to the preprocessor's free list.
   ArgCache = PP.MacroArgCache;
   PP.MacroArgCache = this;
if (isBad) {
       Result = "' '";  // Use something arbitrary, but legal.
     }
   }
 
   PP.CreateString(Result, Tok,
                   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];
-}

cfe/trunk/lib/Lex/TokenLexer.cpp

if (CurTok.isOneOf(tok::hash, tok::hashat)) {
         continue;
       }
       // Else handle the simple argument case.
       SourceLocation ExpansionLocStart =
           getExpansionLocForMacroDefLoc(CurTok.getLocation());
       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
       // the #/#@ operator.
       if (NextTokGetsSpace)
         Res.setFlag(Token::LeadingSpace);
 
       ResultToks.push_back(Res);

cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp

+// 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 <class b> 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);
+}

cfe/trunk/unittests/Lex/LexerTest.cpp

if (tok.is(tok::comma))
       ArgTokens.push_back(Eof);
     else
       ArgTokens.push_back(tok);
   }
 
   auto MacroArgsDeleter = [&PP](MacroArgs *M) { M->destroy(*PP); };
   std::unique_ptr<MacroArgs, decltype(MacroArgsDeleter)> 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
 }
 
 TEST_F(LexerTest, IsNewLineEscapedValid) {
   auto hasNewLineEscaped = [](const char *S) {
     return Lexer::isNewLineEscaped(S, S + strlen(S) - 1);
   };