[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

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

Szelethus marked 3 inline comments as done.
Szelethus added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:469
+    DisplayMacroExpansions =
+        getBooleanOption("expand-macros", /*Default=*/false);
+  return DisplayMacroExpansions.getValue();
----------------
NoQ wrote:
> Should we say something about plists in the option name?
Sure, but should my `AnalyzerOptions` refactoring effort go through first, I intend emit warnings if flags aren't set correctly (eg. the output isn't set to plist but this flag is enabled). Should probably rename `"serialize-stats"` too then.

http://lists.llvm.org/pipermail/cfe-dev/2018-October/059842.html


================
Comment at: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:44-54
+    <dict>
+     <key>kind</key><string>macro_expansion</string>
+     <key>location</key>
+     <dict>
+      <key>line</key><integer>26</integer>
+      <key>col</key><integer>3</integer>
+      <key>file</key><integer>0</integer>
----------------
NoQ wrote:
> Because we're adding an element of an `<array>` rather than a key of a `<dict>`, I'm not entirely sure this is backwards compatible. Clients may crash if they iterate over the `path` array and encounter an unexpected element kind. Is it going to be bad for your use case if we put expansions into a separate array alongside the `path` array?
It shouldn't be :)


https://reviews.llvm.org/D52742