ymandel marked 2 inline comments as done.
ymandel added inline comments.
================
Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:205
if (auto Err = TransformationsOrErr.takeError()) {
- llvm::errs() << "Transformation failed: " << llvm::toString(std::move(Err))
- << "\n";
+ AC.setError(llvm::toString(std::move(Err)));
+ Consumer(AC);
----------------
ilya-biryukov wrote:
> This looks super-complicated.
> Having `Error` in `AtomicChange` seems like a bad idea in the first place,
> why would we choose to use it here?
>
> The following alternatives would encourage clients to handle errors properly:
> 1. accept an `Expected<AtomicChange>` in our callback,
> 2. provide a separate callback to consume errors.
>
> WDYT about picking one of those two?
Agreed! I was using `setError` on the assumption that it was the "standard" way
to express errors. Given that it seems to be totally ignored otherwise, let's
go with option 1. I'll update the revision.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61015/new/
https://reviews.llvm.org/D61015
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
