[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

The fix is great, thank you so much! The test seems to be annoying, it might be worth looking into it some other time, but that is definitely orthogonal to this patch.



================
Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1211-1212
 
-      while (
-          !(ParenthesesDepth == 1 &&
-            (CurrParamII == __VA_ARGS__II ? false : TheTok.is(tok::comma)))) {
+      while (CurrParamII == VariadicParamII || ParenthesesDepth != 1 ||
+             !TheTok.is(tok::comma)) {
         assert(TheTok.isNot(tok::eof) &&
----------------
That looks so much better.


================
Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:529-531
 // FIXME: Stringify and escape __VA_ARGS__ correctly.
 // CHECK: <key>name</key><string>STRINGIFIED_VA_ARGS</string>
 // CHECK-NEXT: <key>expansion</key><string>variadicCFunction(x, "Additional supply depots required.", ")";x = 0;</string>
----------------
These FileCheck lines are great, because the plist file on its own is not very readable, could you include them for the new test as well? Side note, we're already thinking that removing the plist file might be a good idea (its a mess to keep up-to-date, and doesn't test all that much).


================
Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:536
+  do {                       \
+  } while (0)
+
----------------
chrish_ericsson_atx wrote:
> Seems the do {} while (0) idiom is a problem, because the plist output gets formatted differently on windows than it does on Linux.   (Extra space).  
Oh that is a real shame, we absolutely shouldn't do that! Can we somehow provoke a warning through some other way? (null pointer dereference)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87942/new/

https://reviews.llvm.org/D87942