Add a proper algorithm to compute accurate source ranges for diagnostics with

caret locations and source ranges in macros.  Makes ranges more accurate
in some cases, and fixes an assertion failure.

Fixes <rdar://problem/12472249>.

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

diff --git a/lib/Frontend/DiagnosticRenderer.cpp b/lib/Frontend/DiagnosticRenderer.cpp
index d70a11b9c90e708362cd20b353abb293f3cce1db..359b82be60625d23457d84b347eb11341e642a33 100644 (file)
--- a/lib/Frontend/DiagnosticRenderer.cpp
+++ b/lib/Frontend/DiagnosticRenderer.cpp
@@ -18,6 +18,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include <algorithm>
 using namespace clang;
@@ -218,6 +219,53 @@ void DiagnosticRenderer::emitIncludeStackRecursively(SourceLocation Loc,
   emitIncludeLocation(Loc, PLoc, SM);
 }
 
+// Helper function to fix up source ranges.  It takes in an array of ranges,
+// and outputs an array of ranges where we want to draw the range highlighting
+// around the location specified by CaretLoc.
+//
+// To find locations which correspond to the caret, we crawl the macro caller
+// chain for the beginning and end of each range.  If the caret location
+// is in a macro expansion, we search each chain for a location
+// in the same expansion as the caret; otherwise, we crawl to the top of
+// each chain. Two locations are part of the same macro expansion
+// iff the FileID is the same.
+static void mapDiagnosticRanges(
+    SourceLocation CaretLoc,
+    const SmallVectorImpl<CharSourceRange>& Ranges,
+    SmallVectorImpl<CharSourceRange>& SpellingRanges,
+    const SourceManager *SM) {
+  FileID CaretLocFileID = SM->getFileID(CaretLoc);
+
+  for (SmallVectorImpl<CharSourceRange>::const_iterator I = Ranges.begin(),
+       E = Ranges.end();
+       I != E; ++I) {
+    SourceLocation Begin = I->getBegin(), End = I->getEnd();
+    bool IsTokenRange = I->isTokenRange();
+
+    // Search the macro caller chain for the beginning of the range.
+    while (Begin.isMacroID() && SM->getFileID(Begin) != CaretLocFileID)
+      Begin = SM->getImmediateMacroCallerLoc(Begin);
+
+    // Search the macro caller chain for the beginning of the range.
+    while (End.isMacroID() && SM->getFileID(End) != CaretLocFileID) {
+      // The computation of the next End is an inlined version of
+      // getImmediateMacroCallerLoc, except it chooses the end of an
+      // expansion range.
+      if (SM->isMacroArgExpansion(End)) {
+        End = SM->getImmediateSpellingLoc(End);
+      } else {
+        End = SM->getImmediateExpansionRange(End).second;
+      }
+    }
+
+    // Return the spelling location of the beginning and end of the range.
+    Begin = SM->getSpellingLoc(Begin);
+    End = SM->getSpellingLoc(End);
+    SpellingRanges.push_back(CharSourceRange(SourceRange(Begin, End),
+                                             IsTokenRange));
+  }
+}
+
 /// \brief Recursively emit notes for each macro expansion and caret
 /// diagnostics where appropriate.
 ///
@@ -245,9 +293,13 @@ void DiagnosticRenderer::emitMacroExpansionsAndCarets(
   // If this is a file source location, directly emit the source snippet and
   // caret line. Also record the macro depth reached.
   if (Loc.isFileID()) {
+    // Map the ranges.
+    SmallVector<CharSourceRange, 4> SpellingRanges;
+    mapDiagnosticRanges(Loc, Ranges, SpellingRanges, &SM);
+
     assert(MacroDepth == 0 && "We shouldn't hit a leaf node twice!");
     MacroDepth = OnMacroInst;
-    emitCodeContext(Loc, Level, Ranges, Hints, SM);
+    emitCodeContext(Loc, Level, SpellingRanges, Hints, SM);
     return;
   }
   // Otherwise recurse through each macro expansion layer.
@@ -257,8 +309,7 @@ void DiagnosticRenderer::emitMacroExpansionsAndCarets(
   Loc = SM.skipToMacroArgExpansion(Loc);
   
   SourceLocation OneLevelUp = SM.getImmediateMacroCallerLoc(Loc);
-  
-  // FIXME: Map ranges?
+
   emitMacroExpansionsAndCarets(OneLevelUp, Level, Ranges, Hints, SM, MacroDepth,
                                OnMacroInst + 1);
   
@@ -280,17 +331,6 @@ void DiagnosticRenderer::emitMacroExpansionsAndCarets(
   bool Suppressed = (OnMacroInst >= MacroSkipStart &&
                      OnMacroInst < MacroSkipEnd);
   
-  // Map the ranges.
-  for (SmallVectorImpl<CharSourceRange>::iterator I = Ranges.begin(),
-       E = Ranges.end();
-       I != E; ++I) {
-    SourceLocation Start = I->getBegin(), End = I->getEnd();
-    if (Start.isMacroID())
-      I->setBegin(SM.getImmediateMacroCalleeLoc(Start));
-    if (End.isMacroID())
-      I->setEnd(SM.getImmediateMacroCalleeLoc(End));
-  }
-  
   if (Suppressed) {
     // Tell the user that we've skipped contexts.
     if (OnMacroInst == MacroSkipStart) {
@@ -303,14 +343,18 @@ void DiagnosticRenderer::emitMacroExpansionsAndCarets(
     }
     return;
   }
-  
+
+  // Map the ranges.
+  SmallVector<CharSourceRange, 4> SpellingRanges;
+  mapDiagnosticRanges(MacroLoc, Ranges, SpellingRanges, &SM);
+
   SmallString<100> MessageStorage;
   llvm::raw_svector_ostream Message(MessageStorage);
   Message << "expanded from macro '"
           << getImmediateMacroName(MacroLoc, SM, LangOpts) << "'";
   emitDiagnostic(SM.getSpellingLoc(Loc), DiagnosticsEngine::Note,
                  Message.str(),
-                 Ranges, ArrayRef<FixItHint>(), &SM);
+                 SpellingRanges, ArrayRef<FixItHint>(), &SM);
 }
 
 DiagnosticNoteRenderer::~DiagnosticNoteRenderer() {}

diff --git a/lib/Frontend/TextDiagnostic.cpp b/lib/Frontend/TextDiagnostic.cpp
index a421f9ac34ca78d56c4b351bcf7b0fed05953a21..d622fb35f0f0895ca30e79ea5a497486505c53b1 100644 (file)
--- a/lib/Frontend/TextDiagnostic.cpp
+++ b/lib/Frontend/TextDiagnostic.cpp
@@ -1049,16 +1049,8 @@ void TextDiagnostic::highlightRange(const CharSourceRange &R,
                                     const SourceManager &SM) {
   if (!R.isValid()) return;
 
-  SourceLocation Begin = SM.getExpansionLoc(R.getBegin());
-  SourceLocation End = SM.getExpansionLoc(R.getEnd());
-
-  // If the End location and the start location are the same and are a macro
-  // location, then the range was something that came from a macro expansion
-  // or _Pragma.  If this is an object-like macro, the best we can do is to
-  // highlight the range.  If this is a function-like macro, we'd also like to
-  // highlight the arguments.
-  if (Begin == End && R.getEnd().isMacroID())
-    End = SM.getExpansionRange(R.getEnd()).second;
+  SourceLocation Begin = R.getBegin();
+  SourceLocation End = R.getEnd();
 
   unsigned StartLineNo = SM.getExpansionLineNumber(Begin);
   if (StartLineNo > LineNo || SM.getFileID(Begin) != FID)

diff --git a/test/Misc/caret-diags-macros.c b/test/Misc/caret-diags-macros.c
index de1ee76ed1bee73bcd9728ed2f5305b222e2b240..5faddb65f6e68873a820b43e4a019fafa073a942 100644 (file)
--- a/test/Misc/caret-diags-macros.c
+++ b/test/Misc/caret-diags-macros.c
@@ -1,13 +1,13 @@
-// RUN: %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s -strict-whitespace
 
 #define M1(x) x
 #define M2 1;
 void foo() {
   M1(
     M2);
-  // CHECK: :7:{{[0-9]+}}: warning: expression result unused
-  // CHECK: :4:{{[0-9]+}}: note: expanded from macro 'M2'
-  // CHECK: :3:{{[0-9]+}}: note: expanded from macro 'M1'
+  // CHECK: {{.*}}:7:{{[0-9]+}}: warning: expression result unused
+  // CHECK: {{.*}}:4:{{[0-9]+}}: note: expanded from macro 'M2'
+  // CHECK: {{.*}}:3:{{[0-9]+}}: note: expanded from macro 'M1'
 }
 
 #define A 1
@@ -15,10 +15,10 @@ void foo() {
 #define C B
 void bar() {
   C;
-  // CHECK: :17:3: warning: expression result unused
-  // CHECK: :15:11: note: expanded from macro 'C'
-  // CHECK: :14:11: note: expanded from macro 'B'
-  // CHECK: :13:11: note: expanded from macro 'A'
+  // CHECK: {{.*}}:17:3: warning: expression result unused
+  // CHECK: {{.*}}:15:11: note: expanded from macro 'C'
+  // CHECK: {{.*}}:14:11: note: expanded from macro 'B'
+  // CHECK: {{.*}}:13:11: note: expanded from macro 'A'
 }
 
 // rdar://7597492
@@ -40,12 +40,12 @@ void baz(char *Msg) {
 #define macro_many_args3(x, y, z) macro_many_args2(x, y, z)
 
 void test() {
-  macro_args3(1);
+  macro_args3(11);
   // CHECK: {{.*}}:43:15: warning: expression result unused
   // Also check that the 'caret' printing agrees with the location here where
   // its easy to FileCheck.
-  // CHECK-NEXT: macro_args3(1);
-  // CHECK-NEXT: ~~~~~~~~~~~~^~
+  // CHECK-NEXT:      macro_args3(11);
+  // CHECK-NEXT: {{^              \^~}}
   // CHECK: {{.*}}:36:36: note: expanded from macro 'macro_args3'
   // CHECK: {{.*}}:35:36: note: expanded from macro 'macro_args2'
   // CHECK: {{.*}}:34:24: note: expanded from macro 'macro_args1'
@@ -71,13 +71,13 @@ void test() {
 
   macro_many_args3(
     1,
-    macro_args2(2),
+    macro_args2(22),
     3);
   // CHECK: {{.*}}:74:17: warning: expression result unused
   // This caret location needs to be printed *inside* a different macro's
   // arguments.
-  // CHECK-NEXT: macro_args2(2),
-  // CHECK-NEXT: ~~~~~~~~~~~~^~~
+  // CHECK-NEXT:        macro_args2(22),
+  // CHECK-NEXT: {{^                \^~}}
   // CHECK: {{.*}}:35:36: note: expanded from macro 'macro_args2'
   // CHECK: {{.*}}:34:24: note: expanded from macro 'macro_args1'
   // CHECK: {{.*}}:40:55: note: expanded from macro 'macro_many_args3'
@@ -90,10 +90,10 @@ void test() {
 #define variadic_args3(x, y, ...) variadic_args2(x, y, __VA_ARGS__)
 
 void test2() {
-  variadic_args3(1, 2, 3, 4);
+  variadic_args3(1, 22, 3, 4);
   // CHECK: {{.*}}:93:21: warning: expression result unused
-  // CHECK-NEXT: variadic_args3(1, 2, 3, 4);
-  // CHECK-NEXT: ~~~~~~~~~~~~~~~~~~^~~~~~~~
+  // CHECK-NEXT:      variadic_args3(1, 22, 3, 4);
+  // CHECK-NEXT: {{^                    \^~}}
   // CHECK: {{.*}}:90:53: note: expanded from macro 'variadic_args3'
   // CHECK: {{.*}}:89:50: note: expanded from macro 'variadic_args2'
   // CHECK: {{.*}}:88:35: note: expanded from macro 'variadic_args1'
@@ -118,3 +118,48 @@ void test3() {
   // CHECK: {{.*}}:104:70: note: expanded from macro 'variadic_pasting_args2a'
   // CHECK: {{.*}}:102:41: note: expanded from macro 'variadic_pasting_args1'
 }
+
+#define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
+int test4 = BAD_CONDITIONAL_OPERATOR+BAD_CONDITIONAL_OPERATOR;
+// CHECK:         {{.*}}:122:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
+// CHECK-NEXT:    #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
+// CHECK-NEXT: {{^                                      \^}}
+// CHECK:         {{.*}}:122:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
+// CHECK-NEXT:    #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
+// CHECK-NEXT: {{^                                      \^}}
+// CHECK:         {{.*}}:122:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
+// CHECK-NEXT:    #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
+// CHECK-NEXT: {{^                                 ~~~~~\^~~~}}
+
+#define QMARK ?
+#define TWOL (2<
+#define X 1+TWOL 3) QMARK 4:5
+int x = X;
+// CHECK:         {{.*}}:137:9: note: place parentheses around the '+' expression to silence this warning
+// CHECK-NEXT:    int x = X;
+// CHECK-NEXT: {{^        \^}}
+// CHECK-NEXT:    {{.*}}:136:21: note: expanded from macro 'X'
+// CHECK-NEXT:    #define X 1+TWOL 3) QMARK 4:5
+// CHECK-NEXT: {{^          ~~~~~~~~~ \^}}
+// CHECK-NEXT:    {{.*}}:134:15: note: expanded from macro 'QMARK'
+// CHECK-NEXT:    #define QMARK ?
+// CHECK-NEXT: {{^              \^}}
+// CHECK-NEXT:    {{.*}}:137:9: note: place parentheses around the '?:' expression to evaluate it first
+// CHECK-NEXT:    int x = X;
+// CHECK-NEXT: {{^        \^}}
+// CHECK-NEXT:    {{.*}}:136:21: note: expanded from macro 'X'
+// CHECK-NEXT:    #define X 1+TWOL 3) QMARK 4:5
+// CHECK-NEXT: {{^            ~~~~~~~~\^~~~~~~~~}}
+
+#define ONEPLUS 1+
+#define Y ONEPLUS (2<3) QMARK 4:5
+int y = Y;
+// CHECK:         {{.*}}:156:9: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first
+// CHECK-NEXT:    int y = Y;
+// CHECK-NEXT: {{^        \^}}
+// CHECK-NEXT:    {{.*}}:155:25: note: expanded from macro 'Y'
+// CHECK-NEXT:    #define Y ONEPLUS (2<3) QMARK 4:5
+// CHECK-NEXT: {{^          ~~~~~~~~~~~~~ \^}}
+// CHECK-NEXT:    {{.*}}:134:15: note: expanded from macro 'QMARK'
+// CHECK-NEXT:    #define QMARK ?
+// CHECK-NEXT: {{^              \^}}