OSDN Git Service

Revert "[libTooling] In Clang Transformer, change `Metadata` field to deferred evalut...
authorYitzhak Mandelbaum <yitzhakm@google.com>
Mon, 20 Jul 2020 21:24:58 +0000 (21:24 +0000)
committerYitzhak Mandelbaum <yitzhakm@google.com>
Mon, 20 Jul 2020 21:24:58 +0000 (21:24 +0000)
This reverts commit c0b8954ecba500e3d9609152295b74ccd7d89d62.

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

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

index eb6947b..d9e6871 100644 (file)
@@ -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
index a212a86..995bec0 100644 (file)
@@ -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;
index 59b334b..7d6b632 100644 (file)
@@ -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) {