[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.

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

Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.

Uhh, it's such a chore to work on these things -- You can really feel that the preprocessor must've been the first thing implemented in clang. Unfortunately, you really have to counterweight it's shortcomings with excessive amount of comments. I can see an infinite loop and a lot of string comparisons, but someone without having this particular test case as a context, it's very hard to follow what's happening.

Could you please add some code examples in the comments too?



================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1997-1999
         LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
+        if (!LocStart.isMacroID())
+          break;
----------------
> Gets the location of the immediate macro caller, one level up the stack toward the initial macro typed into the source.
Hmm, is there a guarantee that at the end of the stack is not a macro location?


================
Comment at: clang/test/Analysis/diagnostics/macros.cpp:53
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to UINT32_MAX}}
+                        // expected-note at -1 {{Taking false branch}}
----------------
Ah, that looks pretty :D


Repository:
  rC Clang

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

https://reviews.llvm.org/D59121