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

[Gábor Horváth via Phabricator via cfe-commits]

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 expand it.
+    if (Info.Args.count(II)) {
+      for (MacroInfo::tokens_iterator ArgIt = Info.Args[II].begin(),
----------------
Maybe you could use find method to avoid repeated lookups?  (you have 3 identical lookups in the map at the moment).


================
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:886
+        // same way we did above.
+        const auto *II = ArgIt->getIdentifierInfo();
+        if (!II) {
----------------
This `II` shadows the `II` in the outer scope. Maybe it would be better to give names correspond to the named notion, like `ArgII` to avoid confusion. 


https://reviews.llvm.org/D52795