[PATCH] D61015: [LibTooling] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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