Refactoring how macro backtraces are printed

[zhengkai]

Summary

Implement the iteration version of the function emitMacroExpansions instead of the recursion version.

Will work on to improve the diagnostic information in macro.

[rtrieu]

lib/Frontend/DiagnosticRenderer.cpp

403

"bactrace" => "backtrace"

446

This comment was for the recursive version of this code. Change to say that the intermediate macro locations are being stored for processing.

457

There's an extra space at the end of this line, and several below. Remove them.

[zhengkai]

Address the comments/

[zhengkai]

Fix one unchanged in the previous patch.

[rtrieu]

LGTM

include/clang/Frontend/DiagnosticRenderer.h

private:
   void emitIncludeStackRecursively(SourceLocation Loc, const SourceManager &SM);
   void emitImportStack(SourceLocation Loc, const SourceManager &SM);
   void emitImportStackRecursively(SourceLocation Loc, StringRef ModuleName,
                                   const SourceManager &SM);
   void emitModuleBuildStack(const SourceManager &SM);
   void emitCaret(SourceLocation Loc, DiagnosticsEngine::Level Level,
                  ArrayRef<CharSourceRange> Ranges, ArrayRef<FixItHint> Hints,
                  const SourceManager &SM);
+  void emitSingleMacroExpansion(SourceLocation Loc,
+                                DiagnosticsEngine::Level Level,
+                                ArrayRef<CharSourceRange> Ranges,
+                                const SourceManager &SM);
   void emitMacroExpansions(SourceLocation Loc,
                            DiagnosticsEngine::Level Level,
                            ArrayRef<CharSourceRange> Ranges,
                            ArrayRef<FixItHint> Hints,
-                           const SourceManager &SM,
-                           unsigned &MacroDepth,
-                           unsigned OnMacroInst = 0);
+                           const SourceManager &SM);
 public:
   /// \brief Emit a diagnostic.
   ///
   /// This is the primary entry point for emitting diagnostic messages.
   /// It handles formatting and rendering the message as well as any ancillary
   /// information needed based on macros whose expansions impact the
   /// diagnostic.
   ///

lib/Frontend/DiagnosticRenderer.cpp

else {
 
     // Next, emit the actual diagnostic message and caret.
     emitDiagnosticMessage(Loc, PLoc, Level, Message, Ranges, SM, D);
     emitCaret(Loc, Level, MutableRanges, FixItHints, *SM);
 
     // If this location is within a macro, walk from UnexpandedLoc up to Loc
     // and produce a macro backtrace.
     if (UnexpandedLoc.isValid() && UnexpandedLoc.isMacroID()) {
-      unsigned MacroDepth = 0;
-      emitMacroExpansions(UnexpandedLoc, Level, MutableRanges, FixItHints, *SM,
-                          MacroDepth);
+      emitMacroExpansions(UnexpandedLoc, Level, MutableRanges, FixItHints, *SM);
     }
   }
 
   LastLoc = Loc;
   LastLevel = Level;
 
   endDiagnostic(D, Level);
 }
void DiagnosticRenderer::emitCaret(SourceLocation Loc,
                                    ArrayRef<CharSourceRange> Ranges,
                                    ArrayRef<FixItHint> Hints,
                                    const SourceManager &SM) {
   SmallVector<CharSourceRange, 4> SpellingRanges;
   mapDiagnosticRanges(Loc, Ranges, SpellingRanges, &SM);
   emitCodeContext(Loc, Level, SpellingRanges, Hints, SM);
 }
 
+/// \brief A helper function for emitMacroExpansion to print the 
+/// macro expansion message
+void DiagnosticRenderer::emitSingleMacroExpansion(
+    SourceLocation Loc,
+    DiagnosticsEngine::Level Level,
+    ArrayRef<CharSourceRange> Ranges,
+    const SourceManager &SM) {
+  // Find the spelling location for the macro definition. We must use the
+  // spelling location here to avoid emitting a macro bactrace for the note.
+  SourceLocation SpellingLoc = Loc;
+
+  // If this is the expansion of a macro argument, point the caret at the
+  // use of the argument in the definition of the macro, not the expansion.
+  if (SM.isMacroArgExpansion(Loc))
+    SpellingLoc = SM.getImmediateExpansionRange(Loc).first;
+  SpellingLoc = SM.getSpellingLoc(SpellingLoc);
+
+  // Map the ranges into the FileID of the diagnostic location.
+  SmallVector<CharSourceRange, 4> SpellingRanges;
+  mapDiagnosticRanges(Loc, Ranges, SpellingRanges, &SM);
+
+  SmallString<100> MessageStorage;
+  llvm::raw_svector_ostream Message(MessageStorage);
+  StringRef MacroName = getImmediateMacroName(Loc, SM, LangOpts);
+  if (MacroName.empty())
+    Message << "expanded from here";
+  else
+    Message << "expanded from macro '" << MacroName << "'";
+  emitDiagnostic(SpellingLoc, DiagnosticsEngine::Note, Message.str(),
+                SpellingRanges, None, &SM);
+}
+
 /// \brief Recursively emit notes for each macro expansion and caret
 /// diagnostics where appropriate.
 ///
 /// Walks up the macro expansion stack printing expansion notes, the code
 /// snippet, caret, underlines and FixItHint display as appropriate at each
 /// level.
 ///
 /// \param Loc The location for this caret.
 /// \param Level The diagnostic level currently being emitted.
 /// \param Ranges The underlined ranges for this code snippet.
 /// \param Hints The FixIt hints active for this diagnostic.
-/// \param OnMacroInst The current depth of the macro expansion stack.
 void DiagnosticRenderer::emitMacroExpansions(SourceLocation Loc,
                                              DiagnosticsEngine::Level Level,
                                              ArrayRef<CharSourceRange> Ranges,
                                              ArrayRef<FixItHint> Hints,
-                                             const SourceManager &SM,
-                                             unsigned &MacroDepth,
-                                             unsigned OnMacroInst) {
+                                             const SourceManager &SM) {
+
   assert(!Loc.isInvalid() && "must have a valid source location here");
 
   // Walk up to the caller of this macro, and produce a backtrace down to there.
-  SourceLocation OneLevelUp = SM.getImmediateMacroCallerLoc(Loc);
-  if (OneLevelUp.isMacroID())
-    emitMacroExpansions(OneLevelUp, Level, Ranges, Hints, SM,
-                        MacroDepth, OnMacroInst + 1);
-  else
-    MacroDepth = OnMacroInst + 1;
-
-  unsigned MacroSkipStart = 0, MacroSkipEnd = 0;
-  if (MacroDepth > DiagOpts->MacroBacktraceLimit &&
-      DiagOpts->MacroBacktraceLimit != 0) {
-    MacroSkipStart = DiagOpts->MacroBacktraceLimit / 2 +
-    DiagOpts->MacroBacktraceLimit % 2;
-    MacroSkipEnd = MacroDepth - DiagOpts->MacroBacktraceLimit / 2;
-  }
-
-  // Whether to suppress printing this macro expansion.
-  bool Suppressed = (OnMacroInst >= MacroSkipStart &&
-                     OnMacroInst < MacroSkipEnd);
-
-  if (Suppressed) {
-    // Tell the user that we've skipped contexts.
-    if (OnMacroInst == MacroSkipStart) {
-      SmallString<200> MessageStorage;
-      llvm::raw_svector_ostream Message(MessageStorage);
-      Message << "(skipping " << (MacroSkipEnd - MacroSkipStart)
-              << " expansions in backtrace; use -fmacro-backtrace-limit=0 to "
-                 "see all)";
-      emitBasicNote(Message.str());      
-    }
+  SmallVector<SourceLocation, 8> LocationStack;
+  while (Loc.isMacroID()) {
+    LocationStack.push_back(Loc);
+    Loc = SM.getImmediateMacroCallerLoc(Loc);
+    assert(!Loc.isInvalid() && "must have a valid source location here");
+
+  unsigned MacroDepth = LocationStack.size();
+  unsigned MacroLimit = DiagOpts->MacroBacktraceLimit;
+  if (MacroDepth <= MacroLimit || MacroLimit == 0) {
+    for (auto I = LocationStack.rbegin(), E = LocationStack.rend(); 
+         I != E; ++I)
+      emitSingleMacroExpansion(*I, Level, Ranges, SM);
     return;
   }
 
-  // Find the spelling location for the macro definition. We must use the
-  // spelling location here to avoid emitting a macro bactrace for the note.
-  SourceLocation SpellingLoc = Loc;
-  // If this is the expansion of a macro argument, point the caret at the
-  // use of the argument in the definition of the macro, not the expansion.
-  if (SM.isMacroArgExpansion(Loc))
-    SpellingLoc = SM.getImmediateExpansionRange(Loc).first;
-  SpellingLoc = SM.getSpellingLoc(SpellingLoc);
+  unsigned MacroStartMessages = MacroLimit / 2;
+  unsigned MacroEndMessages = MacroLimit / 2 + MacroLimit % 2;
 
-  // Map the ranges into the FileID of the diagnostic location.
-  SmallVector<CharSourceRange, 4> SpellingRanges;
-  mapDiagnosticRanges(Loc, Ranges, SpellingRanges, &SM);
+  for (auto I = LocationStack.rbegin(), 
+            E = LocationStack.rbegin() + MacroStartMessages; 
+       I != E; ++I) 
+    emitSingleMacroExpansion(*I, Level, Ranges, SM);
 
-  SmallString<100> MessageStorage;
+  SmallString<200> MessageStorage;
   llvm::raw_svector_ostream Message(MessageStorage);
-  StringRef MacroName = getImmediateMacroName(Loc, SM, LangOpts);
-  if (MacroName.empty())
-    Message << "expanded from here";
-  else
-    Message << "expanded from macro '" << MacroName << "'";
-  emitDiagnostic(SpellingLoc, DiagnosticsEngine::Note, Message.str(),
-                 SpellingRanges, None, &SM);
+  Message << "(skipping " << (MacroDepth - MacroLimit)
+          << " expansions in backtrace; use -fmacro-backtrace-limit=0 to "
+             "see all)";
+  emitBasicNote(Message.str()); 
+
+  for (auto I = LocationStack.rend() - MacroEndMessages, 
+            E = LocationStack.rend(); 
+       I != E; ++I) 
+    emitSingleMacroExpansion(*I, Level, Ranges, SM);
 }
 
 DiagnosticNoteRenderer::~DiagnosticNoteRenderer() {}
 
 void DiagnosticNoteRenderer::emitIncludeLocation(SourceLocation Loc,
                                                  PresumedLoc PLoc,
                                                  const SourceManager &SM) {
   // Generate a note indicating the include location.