srhines added a comment.
https://reviews.llvm.org/D36806 switches this to cantFail(). Thanks for helping
me better understand LLVM's error strategy.
Repository:
rL LLVM
https://reviews.llvm.org/D36728
___
cfe-commits mailing list
cfe-commits@lis
lhames added a comment.
> Is it ok to drop the assertion in that case (and convert it to a comment)? I
> didn't want to alter too much of this check, since perhaps the original
> author(s) were more skeptical about this breaking (hence the assertion).
> Something like:
>
> // Replacements must
srhines added a comment.
In https://reviews.llvm.org/D36728#842644, @lhames wrote:
> The preferred solution to this is actually to wrap the call with cantFail
> (See
> http://llvm.org/docs/ProgrammersManual.html#using-cantfail-to-simplify-safe-callsites)
> -- it will handle both the assertion
lhames added a comment.
The preferred solution to this is actually to wrap the call with cantFail (See
http://llvm.org/docs/ProgrammersManual.html#using-cantfail-to-simplify-safe-callsites)
-- it will handle both the assertion and consumption of the value for you, and
will simplify calls that r
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310958: Switch to consumeError(), since this can crash
otherwise. (authored by srhines).
Repository:
rL LLVM
https://reviews.llvm.org/D36728
Files:
cfe/trunk/lib/Tooling/Core/Replacement.cpp
Index
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D36728
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
srhines added a comment.
https://reviews.llvm.org/D36729 similarly fixes a few more instances of this
problem (discovered while running tests for this configuration).
https://reviews.llvm.org/D36728
___
cfe-commits mailing list
cfe-commits@lists.ll
