[PATCH] D36728: Switch to consumeError(), since this can crash otherwise.

Stephen Hines via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 15 14:38:01 PDT 2017


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 and consumption of the value for you, and will simplify calls that return an Expected<T>.


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 not conflict since ranges have been merged.
llvm::cantFail(FakeReplaces.add(...));


Repository:
  rL LLVM

https://reviews.llvm.org/D36728





More information about the llvm-commits mailing list