[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

Stephen Hines via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 21 16:28:45 PDT 2017

srhines marked an inline comment as done.
srhines added inline comments.

Comment at: lib/Tooling/Core/Replacement.cpp:505
-    assert(!Err &&
-           "Replacements must not conflict since ranges have been merged.");
-    llvm::consumeError(std::move(Err));
hintonda wrote:
> srhines wrote:
> > hintonda wrote:
> > > While obviously correct, are you concerned that by removing the explanatory text, this change will obscure the reason for the assert?
> > The text is now in a comment above the call.
> Well, that's what I mean.  The reason is no longer in the backtrace.
The backtrace will point to this exact line, so I assume anyone debugging it will eventually read the comment. It might be better to have an optional message to cantFail(), but that isn't within the scope of this change.


More information about the cfe-commits mailing list