christof marked an inline comment as done.
christof added a comment.
Sorry for missing the API comments. Thanks for the fix chapuni :-)
Repository:
rL LLVM
https://reviews.llvm.org/D31709
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht
chapuni added inline comments.
Comment at: cfe/trunk/include/clang/Frontend/DiagnosticRenderer.h:131
/// \param FixItHints The FixIt hints active for this diagnostic.
/// \param SM The SourceManager; will be null if the diagnostic came from the
///frontend, thus
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306384: Revert "Revert "[NFC] Refactor DiagnosticRenderer to
use FullSourceLoc"" (authored by christof).
Changed prior to commit:
https://reviews.llvm.org/D31709?vs=101933&id=104115#toc
Repository:
r
christof added a comment.
I'll go ahead and commit this on the grounds that it was already approved and
the changes I made were the last nit-picks of @rnk
https://reviews.llvm.org/D31709
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:
christof added a comment.
@rnk can you let me know if you are happy with the changes?
@rovka I assume the rebase did not change your mind about the patch. But please
let me know if you have new reservations.
Thanks
Comment at: lib/Frontend/DiagnosticRenderer.cpp:512
// Pro
christof updated this revision to Diff 101933.
christof marked 6 inline comments as done.
christof added a comment.
Rebased onto current HEAD and addressed the comments from Reid.
https://reviews.llvm.org/D31709
Files:
include/clang/Basic/SourceLocation.h
include/clang/Frontend/DiagnosticRe
christof commandeered this revision.
christof added a reviewer: sanwou01.
christof added a comment.
This refactoring was ready to land some time ago, except for a few small
details. It's a shame if we don't commit this refactoring, so with @sanwou01
his agreement, I'm taking over this patch.
sanwou01 added a comment.
Thanks for your comments Reid. Please find my responses inline. I'll spin a new
patch addressing your comments soonish.
Comment at: include/clang/Basic/SourceLocation.h:336
+ bool hasManager() const { return SrcMgr != nullptr; }
/// \pre This Fu
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?
sanwou01 added a comment.
Hi Diana,
Thanks! Will do :)
Sanne
https://reviews.llvm.org/D31709
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rovka accepted this revision.
rovka added a comment.
This revision is now accepted and ready to land.
I don't see anything wrong with this, I think you can commit it in a couple of
days if nobody comes up with a reason why the DiagnosticRenderer shouldn't use
FullSourceLoc.
https://reviews.llv
sanwou01 updated this revision to Diff 95541.
sanwou01 added a comment.
Rebased and clang-formatted.
https://reviews.llvm.org/D31709
Files:
include/clang/Basic/SourceLocation.h
include/clang/Frontend/DiagnosticRenderer.h
include/clang/Frontend/TextDiagnostic.h
lib/Basic/SourceLocation.c
sanwou01 created this revision.
Move the DiagnosticRenderer and its dependents to using FullSourceLocs
instead of a SourceLocation and SourceManager pointer. The changeset is
rather large but entirely mechanical.
This is step one to allow DiagnosticRenderer to take either
llvmn::SMLocs or clang:
13 matches
Mail list logo
