[cfe-commits] r134660 - in /cfe/trunk: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp lib/Frontend/TextDiagnosticPrinter.cpp lib/Lex/TokenLexer.cpp test/Misc/caret-diags-macros.c

Chandler Carruth chandlerc at gmail.com
Thu Jul 7 16:56:37 PDT 2011


Author: chandlerc
Date: Thu Jul  7 18:56:36 2011
New Revision: 134660

URL: http://llvm.org/viewvc/llvm-project?rev=134660&view=rev
Log:
Keep track of which source locations are part of a macro argument
instantiation and improve diagnostics which are stem from macro
arguments to trace the argument itself back through the layers of macro
expansion.

This requires some tricky handling of the source locations, as the
argument appears to be expanded in the opposite direction from the
surrounding macro. This patch provides helper routines that encapsulate
the logic and explain the reasoning behind how we step through macros
during diagnostic printing.

This fixes the rest of the test cases originially in PR9279, and later
split out into PR10214 and PR10215.

There is still some more work we can do here to improve the macro
backtrace, but those will follow as separate patches.

Modified:
    cfe/trunk/include/clang/Basic/SourceManager.h
    cfe/trunk/lib/Basic/SourceManager.cpp
    cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
    cfe/trunk/lib/Lex/TokenLexer.cpp
    cfe/trunk/test/Misc/caret-diags-macros.c

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=134660&r1=134659&r2=134660&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Thu Jul  7 18:56:36 2011
@@ -239,8 +239,11 @@
     /// InstantiationLocStart/InstantiationLocEnd - In a macro expansion, these
     /// indicate the start and end of the instantiation.  In object-like macros,
     /// these will be the same.  In a function-like macro instantiation, the
-    /// start will be the identifier and the end will be the ')'.
+    /// start will be the identifier and the end will be the ')'.  Finally, in
+    /// macro-argument instantitions, the end will be 'SourceLocation()', an
+    /// invalid location.
     unsigned InstantiationLocStart, InstantiationLocEnd;
+
   public:
     SourceLocation getSpellingLoc() const {
       return SourceLocation::getFromRawEncoding(SpellingLoc);
@@ -249,7 +252,9 @@
       return SourceLocation::getFromRawEncoding(InstantiationLocStart);
     }
     SourceLocation getInstantiationLocEnd() const {
-      return SourceLocation::getFromRawEncoding(InstantiationLocEnd);
+      SourceLocation EndLoc =
+        SourceLocation::getFromRawEncoding(InstantiationLocEnd);
+      return EndLoc.isInvalid() ? getInstantiationLocStart() : EndLoc;
     }
 
     std::pair<SourceLocation,SourceLocation> getInstantiationLocRange() const {
@@ -257,19 +262,52 @@
                             getInstantiationLocEnd());
     }
 
-    /// get - Return a InstantiationInfo for an expansion.  IL specifies
-    /// the instantiation location (where the macro is expanded), and SL
-    /// specifies the spelling location (where the characters from the token
-    /// come from).  IL and PL can both refer to normal File SLocs or
+    bool isMacroArgInstantiation() const {
+      // Note that this needs to return false for default constructed objects.
+      return getInstantiationLocStart().isValid() &&
+        SourceLocation::getFromRawEncoding(InstantiationLocEnd).isInvalid();
+    }
+
+    /// create - Return a InstantiationInfo for an expansion. ILStart and
+    /// ILEnd specify the instantiation range (where the macro is expanded),
+    /// and SL specifies the spelling location (where the characters from the
+    /// token come from). All three can refer to normal File SLocs or
     /// instantiation locations.
-    static InstantiationInfo get(SourceLocation ILStart, SourceLocation ILEnd,
-                                 SourceLocation SL) {
+    static InstantiationInfo create(SourceLocation SL,
+                                    SourceLocation ILStart,
+                                    SourceLocation ILEnd) {
       InstantiationInfo X;
       X.SpellingLoc = SL.getRawEncoding();
       X.InstantiationLocStart = ILStart.getRawEncoding();
       X.InstantiationLocEnd = ILEnd.getRawEncoding();
       return X;
     }
+
+    /// createForMacroArg - Return a special InstantiationInfo for the
+    /// expansion of a macro argument into a function-like macro's body. IL
+    /// specifies the instantiation location (where the macro is expanded).
+    /// This doesn't need to be a range because a macro is always instantiated
+    /// at a macro parameter reference, and macro parameters are always exactly
+    /// one token. SL specifies the spelling location (where the characters
+    /// from the token come from). IL and SL can both refer to normal File
+    /// SLocs or instantiation locations.
+    ///
+    /// Given the code:
+    /// \code
+    ///   #define F(x) f(x)
+    ///   F(42);
+    /// \endcode
+    ///
+    /// When expanding '\c F(42)', the '\c x' would call this with an SL
+    /// pointing at '\c 42' anad an IL pointing at its location in the
+    /// definition of '\c F'.
+    static InstantiationInfo createForMacroArg(SourceLocation SL,
+                                               SourceLocation IL) {
+      // We store an intentionally invalid source location for the end of the
+      // instantiation range to mark that this is a macro argument instantation
+      // rather than a normal one.
+      return create(SL, IL, SourceLocation());
+    }
   };
 
   /// SLocEntry - This is a discriminated union of FileInfo and
@@ -533,9 +571,17 @@
     return MainFileID;
   }
 
+  /// createMacroArgInstantiationLoc - Return a new SourceLocation that encodes
+  /// the fact that a token from SpellingLoc should actually be referenced from
+  /// InstantiationLoc, and that it represents the instantiation of a macro
+  /// argument into the function-like macro body.
+  SourceLocation createMacroArgInstantiationLoc(SourceLocation Loc,
+                                                SourceLocation InstantiationLoc,
+                                                unsigned TokLength);
+
   /// createInstantiationLoc - Return a new SourceLocation that encodes the fact
-  /// that a token at Loc should actually be referenced from InstantiationLoc.
-  /// TokLength is the length of the token being instantiated.
+  /// that a token from SpellingLoc should actually be referenced from
+  /// InstantiationLoc.
   SourceLocation createInstantiationLoc(SourceLocation Loc,
                                         SourceLocation InstantiationLocStart,
                                         SourceLocation InstantiationLocEnd,
@@ -746,6 +792,12 @@
     return getDecomposedLoc(SpellingLoc).second;
   }
 
+  /// isMacroArgInstantiation - This method tests whether the given source
+  /// location represents a macro argument's instantiation into the
+  /// function-like macro definition. Such source locations only appear inside
+  /// of the instantiation locations representing where a particular
+  /// function-like macro was expanded.
+  bool isMacroArgInstantiation(SourceLocation Loc) const;
 
   //===--------------------------------------------------------------------===//
   // Queries about the code at a SourceLocation.
@@ -991,6 +1043,14 @@
 private:
   const llvm::MemoryBuffer *getFakeBufferForRecovery() const;
 
+  /// createInstantiationLoc - Implements the common elements of storing an
+  /// instantiation info struct into the SLocEntry table and producing a source
+  /// location that refers to it.
+  SourceLocation createInstantiationLocImpl(const SrcMgr::InstantiationInfo &II,
+                                            unsigned TokLength,
+                                            unsigned PreallocatedID = 0,
+                                            unsigned Offset = 0);
+
   /// isOffsetInFileID - Return true if the specified FileID contains the
   /// specified SourceLocation offset.  This is a very hot method.
   inline bool isOffsetInFileID(FileID FID, unsigned SLocOffset) const {

Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=134660&r1=134659&r2=134660&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Thu Jul  7 18:56:36 2011
@@ -531,16 +531,31 @@
   return LastFileIDLookup = FID;
 }
 
-/// createInstantiationLoc - Return a new SourceLocation that encodes the fact
-/// that a token from SpellingLoc should actually be referenced from
-/// InstantiationLoc.
+SourceLocation
+SourceManager::createMacroArgInstantiationLoc(SourceLocation SpellingLoc,
+                                              SourceLocation ILoc,
+                                              unsigned TokLength) {
+  InstantiationInfo II =
+    InstantiationInfo::createForMacroArg(SpellingLoc, ILoc);
+  return createInstantiationLocImpl(II, TokLength);
+}
+
 SourceLocation SourceManager::createInstantiationLoc(SourceLocation SpellingLoc,
                                                      SourceLocation ILocStart,
                                                      SourceLocation ILocEnd,
                                                      unsigned TokLength,
                                                      unsigned PreallocatedID,
                                                      unsigned Offset) {
-  InstantiationInfo II = InstantiationInfo::get(ILocStart,ILocEnd, SpellingLoc);
+  InstantiationInfo II =
+    InstantiationInfo::create(SpellingLoc, ILocStart, ILocEnd);
+  return createInstantiationLocImpl(II, TokLength, PreallocatedID, Offset);
+}
+
+SourceLocation
+SourceManager::createInstantiationLocImpl(const InstantiationInfo &II,
+                                          unsigned TokLength,
+                                          unsigned PreallocatedID,
+                                          unsigned Offset) {
   if (PreallocatedID) {
     // If we're filling in a preallocated ID, just load in the
     // instantiation entry and return.
@@ -824,6 +839,14 @@
   return Res;
 }
 
+bool SourceManager::isMacroArgInstantiation(SourceLocation Loc) const {
+  if (!Loc.isMacroID()) return false;
+
+  FileID FID = getFileID(Loc);
+  const SrcMgr::SLocEntry *E = &getSLocEntry(FID);
+  const SrcMgr::InstantiationInfo &II = E->getInstantiation();
+  return II.isMacroArgInstantiation();
+}
 
 
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp?rev=134660&r1=134659&r2=134660&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp Thu Jul  7 18:56:36 2011
@@ -292,6 +292,56 @@
   }
 }
 
+/// Look through spelling locations for a macro argument instantiation, and
+/// if found skip to it so that we can trace the argument rather than the macros
+/// in which that argument is used. If no macro argument instantiation is found,
+/// don't skip anything and return the starting location.
+static SourceLocation skipToMacroArgInstantiation(const SourceManager &SM,
+                                                  SourceLocation StartLoc) {
+  for (SourceLocation L = StartLoc; L.isMacroID();
+       L = SM.getImmediateSpellingLoc(L)) {
+    if (SM.isMacroArgInstantiation(L))
+      return L;
+  }
+
+  // Otherwise just return initial location, there's nothing to skip.
+  return StartLoc;
+}
+
+/// Gets the location of the immediate macro caller, one level up the stack
+/// toward the initial macro typed into the source.
+static SourceLocation getImmediateMacroCallerLoc(const SourceManager &SM,
+                                                 SourceLocation Loc) {
+  if (!Loc.isMacroID()) return Loc;
+
+  // When we have the location of (part of) an expanded parameter, its spelling
+  // location points to the argument as typed into the macro call, and
+  // therefore is used to locate the macro caller.
+  if (SM.isMacroArgInstantiation(Loc))
+    return SM.getImmediateSpellingLoc(Loc);
+
+  // Otherwise, the caller of the macro is located where this macro is
+  // instantiated (while the spelling is part of the macro definition).
+  return SM.getImmediateInstantiationRange(Loc).first;
+}
+
+/// Gets the location of the immediate macro callee, one level down the stack
+/// toward the leaf macro.
+static SourceLocation getImmediateMacroCalleeLoc(const SourceManager &SM,
+                                                 SourceLocation Loc) {
+  if (!Loc.isMacroID()) return Loc;
+
+  // When we have the location of (part of) an expanded parameter, its
+  // instantiation location points to the unexpanded paramater reference within
+  // the macro definition (or callee).
+  if (SM.isMacroArgInstantiation(Loc))
+    return SM.getImmediateInstantiationRange(Loc).first;
+
+  // Otherwise, the callee of the macro is located where this location was
+  // spelled inside the macro definition.
+  return SM.getImmediateSpellingLoc(Loc);
+}
+
 void TextDiagnosticPrinter::EmitCaretDiagnostic(SourceLocation Loc,
                                                 CharSourceRange *Ranges,
                                                 unsigned NumRanges,
@@ -312,33 +362,40 @@
     // Whether to suppress printing this macro instantiation.
     bool Suppressed 
       = OnMacroInst >= MacroSkipStart && OnMacroInst < MacroSkipEnd;
-    
-    SourceLocation OneLevelUp = SM.getImmediateInstantiationRange(Loc).first;
-    
+
+    // When processing macros, skip over the instantiations leading up to
+    // a macro argument, and trace the argument's instantiation stack instead.
+    Loc = skipToMacroArgInstantiation(SM, Loc);
+
+    SourceLocation OneLevelUp = getImmediateMacroCallerLoc(SM, Loc);
+
     // FIXME: Map ranges?
     EmitCaretDiagnostic(OneLevelUp, Ranges, NumRanges, SM,
                         Hints, NumHints, Columns,
                         OnMacroInst + 1, MacroSkipStart, MacroSkipEnd);
-    
+
     // Map the location.
-    Loc = SM.getImmediateSpellingLoc(Loc);
+    Loc = getImmediateMacroCalleeLoc(SM, Loc);
 
     // Map the ranges.
     for (unsigned i = 0; i != NumRanges; ++i) {
       CharSourceRange &R = Ranges[i];
       SourceLocation S = R.getBegin(), E = R.getEnd();
       if (S.isMacroID())
-        R.setBegin(SM.getImmediateSpellingLoc(S));
+        R.setBegin(getImmediateMacroCalleeLoc(SM, S));
       if (E.isMacroID())
-        R.setEnd(SM.getImmediateSpellingLoc(E));
+        R.setEnd(getImmediateMacroCalleeLoc(SM, E));
     }
 
     if (!Suppressed) {
+      // Don't print recursive instantiation notes from an instantiation note.
+      Loc = SM.getSpellingLoc(Loc);
+
       // Get the pretty name, according to #line directives etc.
       PresumedLoc PLoc = SM.getPresumedLoc(Loc);
       if (PLoc.isInvalid())
         return;
-      
+
       // If this diagnostic is not in the main file, print out the
       // "included from" lines.
       if (LastWarningLoc != PLoc.getIncludeLoc()) {
@@ -354,9 +411,6 @@
         OS << ' ';
       }
       OS << "note: instantiated from:\n";
-      
-      // Don't print recursive instantiation notes from an instantiation note.
-      Loc = SM.getSpellingLoc(Loc);
 
       EmitCaretDiagnostic(Loc, Ranges, NumRanges, SM, 0, 0,
                           Columns, OnMacroInst + 1, MacroSkipStart,
@@ -772,6 +826,20 @@
   return true;
 }
 
+/// Get the presumed location of a diagnostic message. This computes the
+/// presumed location for the top of any macro backtrace when present.
+static PresumedLoc getDiagnosticPresumedLoc(const SourceManager &SM,
+                                            SourceLocation Loc) {
+  // This is a condensed form of the algorithm used by EmitCaretDiagnostic to
+  // walk to the top of the macro call stack.
+  while (Loc.isMacroID()) {
+    Loc = skipToMacroArgInstantiation(SM, Loc);
+    Loc = getImmediateMacroCallerLoc(SM, Loc);
+  }
+
+  return SM.getPresumedLoc(Loc);
+}
+
 void TextDiagnosticPrinter::HandleDiagnostic(Diagnostic::Level Level,
                                              const DiagnosticInfo &Info) {
   // Default implementation (Warnings/errors count).
@@ -790,7 +858,7 @@
   // if enabled.
   if (Info.getLocation().isValid()) {
     const SourceManager &SM = Info.getSourceManager();
-    PresumedLoc PLoc = SM.getPresumedLoc(Info.getLocation());
+    PresumedLoc PLoc = getDiagnosticPresumedLoc(SM, Info.getLocation());
     if (PLoc.isInvalid()) {
       // At least print the file name if available:
       FileID FID = SM.getFileID(Info.getLocation());
@@ -1043,6 +1111,7 @@
       }
     }
 
+    const SourceManager &SM = LastLoc.getManager();
     unsigned MacroInstSkipStart = 0, MacroInstSkipEnd = 0;
     if (DiagOpts && DiagOpts->MacroBacktraceLimit && !LastLoc.isFileID()) {
       // Compute the length of the macro-instantiation backtrace, so that we
@@ -1051,7 +1120,8 @@
       unsigned Depth = 0;
       do {
         ++Depth;
-        Loc = LastLoc.getManager().getImmediateInstantiationRange(Loc).first;
+        Loc = skipToMacroArgInstantiation(SM, Loc);
+        Loc = getImmediateMacroCallerLoc(SM, Loc);
       } while (!Loc.isFileID());
       
       if (Depth > DiagOpts->MacroBacktraceLimit) {

Modified: cfe/trunk/lib/Lex/TokenLexer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/TokenLexer.cpp?rev=134660&r1=134659&r2=134660&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/TokenLexer.cpp (original)
+++ cfe/trunk/lib/Lex/TokenLexer.cpp Thu Jul  7 18:56:36 2011
@@ -231,9 +231,9 @@
                  "Expected arg identifier to come from definition");
           for (unsigned i = FirstResult, e = ResultToks.size(); i != e; ++i) {
             Token &Tok = ResultToks[i];
-            Tok.setLocation(SM.createInstantiationLoc(Tok.getLocation(),
-                                                      curInst, curInst,
-                                                      Tok.getLength()));
+            Tok.setLocation(SM.createMacroArgInstantiationLoc(Tok.getLocation(),
+                                                              curInst,
+                                                              Tok.getLength()));
           }
         }
 

Modified: cfe/trunk/test/Misc/caret-diags-macros.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/caret-diags-macros.c?rev=134660&r1=134659&r2=134660&view=diff
==============================================================================
--- cfe/trunk/test/Misc/caret-diags-macros.c (original)
+++ cfe/trunk/test/Misc/caret-diags-macros.c Thu Jul  7 18:56:36 2011
@@ -5,11 +5,11 @@
 void foo() {
   M1(
     M2);
-  // CHECK: :6:3: warning: expression result unused
-  // CHECK: :7:5: note: instantiated from:
+  // CHECK: :7:{{[0-9]+}}: warning: expression result unused
+  // CHECK: :4:{{[0-9]+}}: note: instantiated from:
+  // CHECK: :3:{{[0-9]+}}: note: instantiated from:
 }
 
-
 #define A 1
 #define B A
 #define C B
@@ -21,7 +21,6 @@
   // CHECK: :13:11: note: instantiated from:
 }
 
-
 // rdar://7597492
 #define sprintf(str, A, B) \
 __builtin___sprintf_chk (str, 0, 42, A, B)
@@ -31,14 +30,57 @@
 }
 
 
-
-// PR9279 - Notes shouldn't print 'instantiated from' notes recursively.
-#define N1(x) int arr[x]
-#define N2(x) N1(x)
-#define N3(x) N2(x)
-N3(-1);
-
-// CHECK: :39:1: error: 'arr' declared as an array with a negative size
-// CHECK: :38:15: note: instantiated from:
-// CHECK: :37:15: note: instantiated from:
-// CHECK: :39:1: note: instantiated from:
+// PR9279: comprehensive tests for multi-level macro back traces
+#define macro_args1(x) x
+#define macro_args2(x) macro_args1(x)
+#define macro_args3(x) macro_args2(x)
+
+#define macro_many_args1(x, y, z) y
+#define macro_many_args2(x, y, z) macro_many_args1(x, y, z)
+#define macro_many_args3(x, y, z) macro_many_args2(x, y, z)
+
+void test() {
+  macro_args3(1);
+  // 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: {{.*}}:36:36: note: instantiated from:
+  // CHECK: {{.*}}:35:36: note: instantiated from:
+  // CHECK: {{.*}}:34:24: note: instantiated from:
+
+  macro_many_args3(
+    1,
+    2,
+    3);
+  // CHECK: {{.*}}:55:5: warning: expression result unused
+  // CHECK: {{.*}}:40:55: note: instantiated from:
+  // CHECK: {{.*}}:39:55: note: instantiated from:
+  // CHECK: {{.*}}:38:35: note: instantiated from:
+
+  macro_many_args3(
+    1,
+    M2,
+    3);
+  // CHECK: {{.*}}:64:5: warning: expression result unused
+  // CHECK: {{.*}}:4:12: note: instantiated from:
+  // CHECK: {{.*}}:40:55: note: instantiated from:
+  // CHECK: {{.*}}:39:55: note: instantiated from:
+  // CHECK: {{.*}}:38:35: note: instantiated from:
+
+  macro_many_args3(
+    1,
+    macro_args2(2),
+    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: {{.*}}:35:36: note: instantiated from:
+  // CHECK: {{.*}}:34:24: note: instantiated from:
+  // CHECK: {{.*}}:40:55: note: instantiated from:
+  // CHECK: {{.*}}:39:55: note: instantiated from:
+  // CHECK: {{.*}}:38:35: note: instantiated from:
+}





More information about the cfe-commits mailing list