Fix rdar://6849429 - -Wunused-value with deeply nested macro expansion generates...

The "instantiated from" messages coming from the caret diagnostics system are
basically walking the macro expansion tree, emitting each level as it goes.  However, it was
skipping certain leaves in the tree by skipping up the entire instantiation arm every time
it went up one spelling arm.  This caused it to miss some things.  For example, in this
testcase:

#define M1(x) x
#define M2 1;

void foo() {
 M1(M2)
}

we now print:

/Users/sabre/Desktop/clang-unused-value-macro.c:6:2: warning: expression result unused

 M1(M2)
 ^~~~~~
/Users/sabre/Desktop/clang-unused-value-macro.c:6:5: note: instantiated from:

 M1(M2)
    ^~
/Users/sabre/Desktop/clang-unused-value-macro.c:3:12: note: instantiated from:

#define M2 1;
           ^

Previously we didn't print the last line, so we never emitted the caret pointing to the 1!

Incidentally, the spaces between the lines is really noisy, I think we should reconsider
this heuristic (which adds them when the printed code starts too close to the start of the
line).

The regression test can't use -verify, because -verify doesn't catch notes for macro
instantiation history.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@71025 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Frontend/TextDiagnosticPrinter.cpp b/lib/Frontend/TextDiagnosticPrinter.cpp
index e8492ea0a8dd40570cc733afed56580ff4fde852..90963c82aca41112f92ab1bf2481d66d88991632 100644 (file)
--- a/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -14,6 +14,7 @@
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/SmallString.h"
 #include <algorithm>
@@ -245,37 +246,40 @@ void TextDiagnosticPrinter::EmitCaretDiagnostic(SourceLocation Loc,
                                                 unsigned AvoidColumn,
                                                 unsigned Columns) {
   assert(!Loc.isInvalid() && "must have a valid source location here");
-  
-  // We always emit diagnostics about the instantiation points, not the spelling
-  // points.  This more closely correlates to what the user writes.
+
+  // If this is a macro ID, first emit information about where this was
+  // instantiated (recursively) then emit information about where. the token was
+  // spelled from.
   if (!Loc.isFileID()) {
     SourceLocation OneLevelUp = SM.getImmediateInstantiationRange(Loc).first;
+    // FIXME: Map ranges?
     EmitCaretDiagnostic(OneLevelUp, Ranges, NumRanges, SM, 0, 0, AvoidColumn,
                         Columns);
-    
-    // Map the location through the macro.
-    Loc = SM.getInstantiationLoc(SM.getImmediateSpellingLoc(Loc));
 
+    Loc = SM.getImmediateSpellingLoc(Loc);
+    
     // Map the ranges.
     for (unsigned i = 0; i != NumRanges; ++i) {
       SourceLocation S = Ranges[i].getBegin(), E = Ranges[i].getEnd();
-      if (S.isMacroID())
-        S = SM.getInstantiationLoc(SM.getImmediateSpellingLoc(S));
-      if (E.isMacroID())
-        E = SM.getInstantiationLoc(SM.getImmediateSpellingLoc(E));
+      if (S.isMacroID()) S = SM.getImmediateSpellingLoc(S);
+      if (E.isMacroID()) E = SM.getImmediateSpellingLoc(E);
       Ranges[i] = SourceRange(S, E);
     }
     
     if (ShowLocation) {
+      std::pair<FileID, unsigned> IInfo = SM.getDecomposedInstantiationLoc(Loc);
+      
       // Emit the file/line/column that this expansion came from.
-      OS << SM.getBufferName(Loc) << ':' << SM.getInstantiationLineNumber(Loc)
-         << ':';
+      OS << SM.getBuffer(IInfo.first)->getBufferIdentifier() << ':'
+         << SM.getLineNumber(IInfo.first, IInfo.second) << ':';
       if (ShowColumn)
-        OS << SM.getInstantiationColumnNumber(Loc) << ':';
+        OS << SM.getColumnNumber(IInfo.first, IInfo.second) << ':';
       OS << ' ';
     }
     OS << "note: instantiated from:\n";
-    AvoidColumn = 0;
+    
+    EmitCaretDiagnostic(Loc, Ranges, NumRanges, SM, Hints, NumHints, 0,Columns);
+    return;
   }
   
   // Decompose the location into a FID/Offset pair.

diff --git a/test/Misc/caret-diags-macros.c b/test/Misc/caret-diags-macros.c
new file mode 100644 (file)
index 0000000..0a5aa07
--- /dev/null
+++ b/test/Misc/caret-diags-macros.c
@@ -0,0 +1,26 @@
+// RUN: clang-cc -fsyntax-only %s >& %t &&
+
+#define M1(x) x
+
+// RUN: grep ":6:12: note: instantiated from:" %t &&
+#define M2 1;
+
+void foo() {
+ // RUN: grep ":10:2: warning: expression result unused" %t &&
+ M1(
+ // RUN: grep ":12:5: note: instantiated from:" %t &&
+    M2)
+}
+
+// RUN: grep ":16:11: note: instantiated from:" %t &&
+#define A 1
+// RUN: grep ":18:11: note: instantiated from:" %t &&
+#define B A
+// RUN: grep ":20:11: note: instantiated from:" %t &&
+#define C B
+
+void bar() {
+  // RUN: grep  ":24:3: warning: expression result unused" %t
+  C;
+}
+