[clang] e334f04 - [libTooling] Support TransformerResult<void> in consumer callbacks
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 28 08:41:31 PDT 2022
Author: Eric Li
Date: 2022-03-28T15:39:46Z
New Revision: e334f044cdb5c0f4f549fc0380a6757e37ccb105
URL: https://github.com/llvm/llvm-project/commit/e334f044cdb5c0f4f549fc0380a6757e37ccb105
DIFF: https://github.com/llvm/llvm-project/commit/e334f044cdb5c0f4f549fc0380a6757e37ccb105.diff
LOG: [libTooling] Support TransformerResult<void> in consumer callbacks
Support `TransformerResult<void>` in the consumer callback, which
allows generic code to more naturally use the `Transformer` interface
(instead of needing to specialize on `void`).
This also delete the specialization that existed within `Transformer`
itself, instead replacing it with an `std::function` adapter.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D122499
Added:
Modified:
clang/include/clang/Tooling/Transformer/Transformer.h
clang/lib/Tooling/Transformer/Transformer.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/Transformer/Transformer.h b/clang/include/clang/Tooling/Transformer/Transformer.h
index db2feac06d4aa..4e93783f2b9d9 100644
--- a/clang/include/clang/Tooling/Transformer/Transformer.h
+++ b/clang/include/clang/Tooling/Transformer/Transformer.h
@@ -21,7 +21,7 @@ namespace tooling {
namespace detail {
/// Implementation details of \c Transformer with type erasure around
-/// \c RewriteRule and \c RewriteRule<T> as well as the corresponding consumers.
+/// \c RewriteRule<T> as well as the corresponding consumers.
class TransformerImpl {
public:
virtual ~TransformerImpl() = default;
@@ -43,33 +43,6 @@ class TransformerImpl {
onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) = 0;
};
-/// Implementation for when no metadata is generated as a part of the
-/// \c RewriteRule.
-class NoMetadataImpl final : public TransformerImpl {
- transformer::RewriteRule Rule;
- std::function<void(Expected<llvm::MutableArrayRef<AtomicChange>>)> Consumer;
-
-public:
- explicit NoMetadataImpl(
- transformer::RewriteRule R,
- std::function<void(Expected<llvm::MutableArrayRef<AtomicChange>>)>
- Consumer)
- : Rule(std::move(R)), Consumer(std::move(Consumer)) {
- assert(llvm::all_of(Rule.Cases,
- [](const transformer::RewriteRule::Case &Case) {
- return Case.Edits;
- }) &&
- "edit generator must be provided for each rule");
- }
-
-private:
- void onMatchImpl(const ast_matchers::MatchFinder::MatchResult &Result) final;
- std::vector<ast_matchers::internal::DynTypedMatcher>
- buildMatchers() const final {
- return transformer::detail::buildMatchers(Rule);
- }
-};
-
// FIXME: Use std::type_identity or backport when available.
template <class T> struct type_identity {
using type = T;
@@ -81,8 +54,6 @@ template <typename T> struct TransformerResult {
T Metadata;
};
-// Specialization provided only to avoid SFINAE on the Transformer
-// constructor; not intended for use.
template <> struct TransformerResult<void> {
llvm::MutableArrayRef<AtomicChange> Changes;
};
@@ -107,8 +78,14 @@ class Transformer : public ast_matchers::MatchFinder::MatchCallback {
/// other.
explicit Transformer(transformer::RewriteRuleWith<void> Rule,
ChangeSetConsumer Consumer)
- : Impl(std::make_unique<detail::NoMetadataImpl>(std::move(Rule),
- std::move(Consumer))) {}
+ : Transformer(std::move(Rule),
+ [Consumer = std::move(Consumer)](
+ llvm::Expected<TransformerResult<void>> Result) {
+ if (Result)
+ Consumer(Result->Changes);
+ else
+ Consumer(Result.takeError());
+ }) {}
/// \param Consumer receives all rewrites and the associated metadata for a
/// single match, or an error. Will always be called for each match, even if
@@ -135,6 +112,44 @@ class Transformer : public ast_matchers::MatchFinder::MatchCallback {
};
namespace detail {
+/// Asserts that all \c Metadata for the \c Rule is set.
+/// FIXME: Use constexpr-if in C++17.
+/// @{
+template <typename T>
+void assertMetadataSet(const transformer::RewriteRuleWith<T> &Rule) {
+ assert(llvm::all_of(Rule.Metadata,
+ [](const typename transformer::Generator<T> &Metadata)
+ -> bool { return !!Metadata; }) &&
+ "metadata generator must be provided for each rule");
+}
+template <>
+inline void assertMetadataSet(const transformer::RewriteRuleWith<void> &) {}
+/// @}
+
+/// Runs the metadata generator on \c Rule and stuffs it into \c Result.
+/// FIXME: Use constexpr-if in C++17.
+/// @{
+template <typename T>
+llvm::Error
+populateMetadata(const transformer::RewriteRuleWith<T> &Rule,
+ size_t SelectedCase,
+ const ast_matchers::MatchFinder::MatchResult &Match,
+ TransformerResult<T> &Result) {
+ auto Metadata = Rule.Metadata[SelectedCase]->eval(Match);
+ if (!Metadata)
+ return Metadata.takeError();
+ Result.Metadata = std::move(*Metadata);
+ return llvm::Error::success();
+}
+template <>
+inline llvm::Error
+populateMetadata(const transformer::RewriteRuleWith<void> &, size_t,
+ const ast_matchers::MatchFinder::MatchResult &Match,
+ TransformerResult<void> &) {
+ return llvm::Error::success();
+}
+/// @}
+
/// Implementation when metadata is generated as a part of the rewrite. This
/// happens when we have a \c RewriteRuleWith<T>.
template <typename T> class WithMetadataImpl final : public TransformerImpl {
@@ -150,10 +165,7 @@ template <typename T> class WithMetadataImpl final : public TransformerImpl {
[](const transformer::RewriteRuleBase::Case &Case)
-> bool { return !!Case.Edits; }) &&
"edit generator must be provided for each rule");
- assert(llvm::all_of(Rule.Metadata,
- [](const typename transformer::Generator<T> &Metadata)
- -> bool { return !!Metadata; }) &&
- "metadata generator must be provided for each rule");
+ assertMetadataSet(Rule);
}
private:
@@ -174,16 +186,19 @@ template <typename T> class WithMetadataImpl final : public TransformerImpl {
Consumer(C.takeError());
return;
}
+ } else if (std::is_void<T>::value) {
+ // If we don't have metadata and we don't have any edits, skip.
+ return;
}
- auto Metadata = Rule.Metadata[I]->eval(Result);
- if (!Metadata) {
- Consumer(Metadata.takeError());
+ TransformerResult<T> RewriteResult;
+ if (auto E = populateMetadata(Rule, I, Result, RewriteResult)) {
+ Consumer(std::move(E));
return;
}
- Consumer(TransformerResult<T>{llvm::MutableArrayRef<AtomicChange>(Changes),
- std::move(*Metadata)});
+ RewriteResult.Changes = llvm::MutableArrayRef<AtomicChange>(Changes);
+ Consumer(std::move(RewriteResult));
}
std::vector<ast_matchers::internal::DynTypedMatcher>
diff --git a/clang/lib/Tooling/Transformer/Transformer.cpp b/clang/lib/Tooling/Transformer/Transformer.cpp
index 1cf824d2cbe49..f95f2ab7d954d 100644
--- a/clang/lib/Tooling/Transformer/Transformer.cpp
+++ b/clang/lib/Tooling/Transformer/Transformer.cpp
@@ -66,26 +66,6 @@ TransformerImpl::convertToAtomicChanges(
return Changes;
}
-void NoMetadataImpl::onMatchImpl(const MatchFinder::MatchResult &Result) {
- size_t I = transformer::detail::findSelectedCase(Result, Rule);
- auto Transformations = Rule.Cases[I].Edits(Result);
- if (!Transformations) {
- Consumer(Transformations.takeError());
- return;
- }
-
- if (Transformations->empty())
- return;
-
- auto Changes = convertToAtomicChanges(*Transformations, Result);
- if (!Changes) {
- Consumer(Changes.takeError());
- return;
- }
-
- Consumer(llvm::MutableArrayRef<AtomicChange>(*Changes));
-}
-
} // namespace detail
void Transformer::registerMatchers(MatchFinder *MatchFinder) {
More information about the cfe-commits
mailing list