[PATCH] D31709: [NFC] Refactor DiagnosticRenderer to use FullSourceLoc

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: include/clang/Basic/SourceLocation.h:336
 
+  bool hasManager() const { return SrcMgr != nullptr; }
   /// \pre This FullSourceLoc has an associated SourceManager.
----------------
SrcMgr is only non-null when the location is invalid, right? Can you do something like:
  bool hasManager() const {
    bool R = SrcMgr != nullptr;
    assert(R == isValid() && "FullSourceLoc has location but no manager");
    return R;
  }


================
Comment at: lib/Basic/SourceLocation.cpp:25
 
+namespace clang {
 void PrettyStackTraceLoc::print(raw_ostream &OS) const {
----------------
This doesn't seem necessary, you aren't defining any free functions, right?


================
Comment at: lib/Frontend/DiagnosticRenderer.cpp:139
 void DiagnosticRenderer::emitStoredDiagnostic(StoredDiagnostic &Diag) {
-  emitDiagnostic(Diag.getLocation(), Diag.getLevel(), Diag.getMessage(),
-                 Diag.getRanges(), Diag.getFixIts(),
-                 Diag.getLocation().isValid() ? &Diag.getLocation().getManager()
-                                              : nullptr,
-                 &Diag);
+  emitDiagnostic(
+      Diag.getLocation().isValid()
----------------
I think `Diag.getLocation()` is already a FullSourceLoc, no need to check it.


================
Comment at: lib/Frontend/DiagnosticRenderer.cpp:512
   // Produce a stack of macro backtraces.
-  SmallVector<SourceLocation, 8> LocationStack;
+  SmallVector<FullSourceLoc, 8> LocationStack;
   unsigned IgnoredEnd = 0;
----------------
This seems inefficient, it wastes space on `SourceManager` pointers that will all be the same.


https://reviews.llvm.org/D31709