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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
