[PATCH] D36806: Switch to cantFail(), since it does the same assertion.
don hinton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 21 16:45:30 PDT 2017
hintonda 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));
----------------
srhines wrote:
> 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.
Sorry, I meant the output of llvm::sys::PrintStackTrace(), which include the assert.
https://reviews.llvm.org/D36806
More information about the cfe-commits
mailing list