[clang] bd994b8 - Revert "[libTooling] In Clang Transformer, change `Metadata` field to deferred evalutaion"

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-07-20T21:24:58Z
New Revision: bd994b81d376c7132b1155c31e99ce27a08f7ba3

URL: https://github.com/llvm/llvm-project/commit/bd994b81d376c7132b1155c31e99ce27a08f7ba3
DIFF: https://github.com/llvm/llvm-project/commit/bd994b81d376c7132b1155c31e99ce27a08f7ba3.diff

LOG: Revert "[libTooling] In Clang Transformer, change `Metadata` field to deferred evalutaion"

This reverts commit c0b8954ecba500e3d9609152295b74ccd7d89d62.

The commit has broken various builds. Reverting while I investigate the cause.

Added: 
    

Modified: 
    clang/include/clang/Tooling/Transformer/RewriteRule.h
    clang/lib/Tooling/Transformer/RewriteRule.cpp
    clang/unittests/Tooling/TransformerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index eb6947b54885..d9e68717d5c8 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -47,8 +47,6 @@ using EditGenerator = MatchConsumer<llvm::SmallVector<Edit, 1>>;
 
 using TextGenerator = std::shared_ptr<MatchComputation<std::string>>;
 
-using AnyGenerator = MatchConsumer<llvm::Any>;
-
 // Description of a source-code edit, expressed in terms of an AST node.
 // Includes: an ID for the (bound) node, a selector for source related to the
 // node, a replacement and, optionally, an explanation for the edit.
@@ -89,11 +87,7 @@ struct ASTEdit {
   RangeSelector TargetRange;
   TextGenerator Replacement;
   TextGenerator Note;
-  // Not all transformations will want or need to attach metadata and therefore
-  // should not be required to do so.
-  AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &) {
-    return llvm::Any();
-  };
+  llvm::Any Metadata;
 };
 
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
@@ -267,27 +261,9 @@ inline ASTEdit insertAfter(RangeSelector S, TextGenerator Replacement) {
 /// Removes the source selected by \p S.
 ASTEdit remove(RangeSelector S);
 
-// 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 =
-      [Gen = std::move(Metadata)](
-          const ast_matchers::MatchFinder::MatchResult &R) -> llvm::Any {
-    return Gen(R);
-  };
-
-  return Edit;
+inline ASTEdit withMetadata(ASTEdit edit, llvm::Any Metadata) {
+  edit.Metadata = std::move(Metadata);
+  return edit;
 }
 
 /// The following three functions are a low-level part of the RewriteRule

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index a212a868c81d..995bec03cd66 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -44,13 +44,10 @@ translateEdits(const MatchResult &Result, ArrayRef<ASTEdit> ASTEdits) {
     auto Replacement = E.Replacement->eval(Result);
     if (!Replacement)
       return Replacement.takeError();
-    auto Metadata = E.Metadata(Result);
-    if (!Metadata)
-      return Metadata.takeError();
     transformer::Edit T;
     T.Range = *EditRange;
     T.Replacement = std::move(*Replacement);
-    T.Metadata = std::move(*Metadata);
+    T.Metadata = E.Metadata;
     Edits.push_back(std::move(T));
   }
   return Edits;

diff  --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp
index 59b334b0ea5a..7d6b63293748 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -440,12 +440,6 @@ TEST_F(TransformerTest, RemoveEdit) {
 }
 
 TEST_F(TransformerTest, WithMetadata) {
-  auto makeMetadata = [](const MatchFinder::MatchResult &R) -> llvm::Any {
-    int N =
-        R.Nodes.getNodeAs<IntegerLiteral>("int")->getValue().getLimitedValue();
-    return N;
-  };
-
   std::string Input = R"cc(
     int f() {
       int x = 5;
@@ -454,11 +448,8 @@ TEST_F(TransformerTest, WithMetadata) {
   )cc";
 
   Transformer T(
-      makeRule(
-          declStmt(containsDeclaration(0, varDecl(hasInitializer(
-                                              integerLiteral().bind("int")))))
-              .bind("decl"),
-          withMetadata(remove(statement(std::string("decl"))), makeMetadata)),
+      makeRule(declStmt().bind("decl"),
+               withMetadata(remove(statement(std::string("decl"))), 17)),
       consumer());
   T.registerMatchers(&MatchFinder);
   auto Factory = newFrontendActionFactory(&MatchFinder);
@@ -468,7 +459,7 @@ TEST_F(TransformerTest, WithMetadata) {
   ASSERT_EQ(Changes.size(), 1u);
   const llvm::Any &Metadata = Changes[0].getMetadata();
   ASSERT_TRUE(llvm::any_isa<int>(Metadata));
-  EXPECT_THAT(llvm::any_cast<int>(Metadata), 5);
+  EXPECT_THAT(llvm::any_cast<int>(Metadata), 17);
 }
 
 TEST_F(TransformerTest, MultiChange) {