[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

Szelethus added a comment. Ping, @xazax.hun, any objections? https://reviews.llvm.org/D52795 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

xazax.hun accepted this revision. xazax.hun added a reviewer: xazax.hun. xazax.hun added a comment. Some minor comment inline. Otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:879 +// If this token is the current macro's argument, we should exp

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

NoQ accepted this revision. NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677 +/// need to expanded further when it is nested inside another macro. +class MacroArgMap : public std::map { +public: Szelethus wrote: > Szelethus

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:685 const MacroInfo *MI = nullptr; + llvm::Optional Args; I wonder if the optional gives us any value here. An empty map could be just as great to represent that ther

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

Szelethus marked 6 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677 +/// need to expanded further when it is nested inside another macro. +class MacroArgMap : public std::map { +public: NoQ wro

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

xazax.hun added a comment. I would love to see a test with deeper macro in macro expansion and larger number of arguments, with some of the arguments unused. Some minor nits inline, otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:831 const Ma

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1018 +auto It = CurrExpArgTokens.begin(); +while (It != CurrExpArgTokens.end()) { + if (It->isNot(tok::identifier)) { xazax.hun wrote: > Maybe a for loop mor n

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

Szelethus added a comment. Herald added a subscriber: gamesh411. Unfortunately, I found //yet another// corner case I didn't cover: if the macro arguments themselves are macros. I already fixed it, but it raises the question that what other things I may have missed? I genuinely dislike this proj

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

Szelethus marked 5 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:982 + int ParenthesesDepth = 1; + while (ParenthesesDepth != 0) { ++It; xazax.hun wrote: > Is the misspelling already comm

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677 +/// need to expanded further when it is nested inside another macro. +class Ma

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677 +/// need to expanded further when it is nested inside another macro. +class MacroArgMap : public std::map { +public: george.karpenkov wrote: > Please don't do this, inh

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677 +/// need to expanded further when it is nested inside another macro. +class MacroArgMap : public std::map { +public: Szelethus wrote: > george.karpenkov wrote: > > Plea