Skip to content

Commit

Permalink
Revert "[libTooling] In Clang Transformer, change Metadata field to…
Browse files Browse the repository at this point in the history
… deferred evalutaion"

This reverts commit c0b8954.

The commit has broken various builds. Reverting while I investigate the cause.
  • Loading branch information
ymand committed Jul 20, 2020
1 parent c0b8954 commit bd994b8
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 44 deletions.
32 changes: 4 additions & 28 deletions clang/include/clang/Tooling/Transformer/RewriteRule.h
Expand Up @@ -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.
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions clang/lib/Tooling/Transformer/RewriteRule.cpp
Expand Up @@ -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;
Expand Down
15 changes: 3 additions & 12 deletions clang/unittests/Tooling/TransformerTest.cpp
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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) {
Expand Down

0 comments on commit bd994b8

Please sign in to comment.