[PATCH] D83820: Change metadata to deferred evalutaion in Clang Transformer.

[Andy Soffer via Phabricator via cfe-commits]

asoffer updated this revision to Diff 278262.
asoffer added a comment.

Add comments describing why we provide defaults for Metadata generation and design of withMetadata function template.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83820/new/

https://reviews.llvm.org/D83820

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h


Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===================================================================
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -89,6 +89,8 @@
   RangeSelector TargetRange;
   TextGenerator Replacement;
   TextGenerator Note;
+  // Not all transformations will want or need to attach metadata and therefore
+  // sholud not be requierd to do so.
   AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &) {
     return llvm::Any();
   };
@@ -265,7 +267,18 @@
 /// Removes the source selected by \p S.
 ASTEdit remove(RangeSelector S);
 
-// TODO(asoffer): If this returns an llvm::Expected, should we unwrap?
+// FIXME: If `Metadata` returns an `llvm::Expected<T>` the `AnyGenerator` will
+// construct an `llvm::Expected<llvm::Any>` where no error is present but the
+// `llvm::Any` holds the error. This is unlikely but potentially surprising.
+// Perhaps the `llvm::Expected` should be unwrapped, or perhaps this should be a
+// compile-time error. No solution here is perfect.
+//
+// Note: This function template accepts any type callable with a MatchResult
+// rather than a `std::function` because the return-type needs to be deduced. If
+// it accepted a `std::function<R(MatchResult)>`, lambdas or other callable types
+// would not be able to deduce `R`, and users would be forced to specify
+// explicitly the type they intended to return by wrapping the lambda at the
+// call-site.
 template <typename Callable>
 inline ASTEdit withMetadata(ASTEdit Edit, Callable Metadata) {
   Edit.Metadata =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83820.278262.patch
Type: text/x-patch
Size: 1647 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200715/33015428/attachment.bin>