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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits