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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 192765.
NoQ added a comment.

An elegant solution for a more civilized age. Unfortunately, doesn't preserve the `UINT32_MAX` macro in the newly added test - i'll try a bit harder to preserve it. Relies on D59977 <https://reviews.llvm.org/D59977> to work.


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

https://reviews.llvm.org/D59121

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/macros.cpp


Index: clang/test/Analysis/diagnostics/macros.cpp
===================================================================
--- clang/test/Analysis/diagnostics/macros.cpp
+++ clang/test/Analysis/diagnostics/macros.cpp
@@ -47,3 +47,14 @@
                     // expected-note at -1 {{Dereference of null pointer (loaded from variable 'p')}}
   }
 }
+
+#define nested_null_split(x) if ((x) != UINT32_MAX) {}
+
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to -1}}
+                        // expected-note at -1 {{Taking false branch}}
+  if (!p) // expected-note {{Assuming 'p' is null}}
+          // expected-note at -1 {{Taking true branch}}
+    *p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+            // expected-note at -1 {{Dereference of null pointer (loaded from variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1969,43 +1969,22 @@
   const Expr *OriginalExpr = Ex;
   Ex = Ex->IgnoreParenCasts();
 
-  // Use heuristics to determine if Ex is a macro expending to a literal and
-  // if so, use the macro's name.
-  SourceLocation LocStart = Ex->getBeginLoc();
-  SourceLocation LocEnd = Ex->getEndLoc();
-  if (LocStart.isMacroID() && LocEnd.isMacroID() &&
-      (isa<GNUNullExpr>(Ex) ||
-       isa<ObjCBoolLiteralExpr>(Ex) ||
-       isa<CXXBoolLiteralExpr>(Ex) ||
-       isa<IntegerLiteral>(Ex) ||
-       isa<FloatingLiteral>(Ex))) {
-    StringRef StartName = Lexer::getImmediateMacroNameForDiagnostics(LocStart,
-      BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
-    StringRef EndName = Lexer::getImmediateMacroNameForDiagnostics(LocEnd,
-      BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
-    bool beginAndEndAreTheSameMacro = StartName.equals(EndName);
-
-    bool partOfParentMacro = false;
-    if (ParentEx->getBeginLoc().isMacroID()) {
-      StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
-          ParentEx->getBeginLoc(), BRC.getSourceManager(),
-          BRC.getASTContext().getLangOpts());
-      partOfParentMacro = PName.equals(StartName);
-    }
-
-    if (beginAndEndAreTheSameMacro && !partOfParentMacro ) {
-      // Get the location of the macro name as written by the caller.
-      SourceLocation Loc = LocStart;
-      while (LocStart.isMacroID()) {
-        Loc = LocStart;
-        LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
+  if (isa<GNUNullExpr>(Ex) || isa<ObjCBoolLiteralExpr>(Ex) ||
+      isa<CXXBoolLiteralExpr>(Ex) || isa<IntegerLiteral>(Ex) ||
+      isa<FloatingLiteral>(Ex)) {
+    // Use heuristics to determine if the expression is a macro
+    // expanding to a literal and if so, use the macro's name.
+    SourceLocation LocStart = OriginalExpr->getBeginLoc();
+    SourceLocation LocEnd = OriginalExpr->getEndLoc();
+    if (LocStart.isMacroID() && LocEnd.isMacroID()) {
+      SourceManager &SM = BRC.getSourceManager();
+      const LangOptions &LO = BRC.getASTContext().getLangOpts();
+      if (Lexer::isAtStartOfMacroExpansion(LocStart, SM, LO) &&
+          Lexer::isAtEndOfMacroExpansion(LocEnd, SM, LO)) {
+        SourceRange R{LocStart, LocEnd};
+        Out << Lexer::getSourceText(Lexer::getAsCharRange(R, SM, LO), SM, LO);
+        return false;
       }
-      StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
-        Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
-
-      // Return the macro name.
-      Out << MacroName;
-      return false;
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59121.192765.patch
Type: text/x-patch
Size: 3759 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190329/ff1a904a/attachment-0001.bin>