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 ↗

(On Diff #30740)

"bactrace" => "backtrace"

446 ↗

(On Diff #30740)

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

457 ↗

(On Diff #30740)

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

cfe/trunk/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.
   ///

cfe/trunk/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 backtrace 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());      
-    }
+  // Produce a stack of macro backtraces.
+  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.