[cfe-commits] r71025 - in /cfe/trunk: lib/Frontend/TextDiagnosticPrinter.cpp test/Misc/caret-diags-macros.c

[Chris Lattner]

Author: lattner
Date: Tue May  5 17:03:18 2009
New Revision: 71025

URL: http://llvm.org/viewvc/llvm-project?rev=71025&view=rev
Log:
Fix rdar://6849429 - -Wunused-value with deeply nested macro expansion generates untraceable warnings

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.


Added:
    cfe/trunk/test/Misc/caret-diags-macros.c
Modified:
    cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp

Modified: cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp?rev=71025&r1=71024&r2=71025&view=diff

==============================================================================
--- cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp Tue May  5 17:03:18 2009
@@ -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 @@
                                                 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.

Added: cfe/trunk/test/Misc/caret-diags-macros.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/caret-diags-macros.c?rev=71025&view=auto

==============================================================================
--- cfe/trunk/test/Misc/caret-diags-macros.c (added)
+++ cfe/trunk/test/Misc/caret-diags-macros.c Tue May  5 17:03:18 2009
@@ -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;
+}
+