From e334f044cdb5c0f4f549fc0380a6757e37ccb105 Mon Sep 17 00:00:00 2001 From: Eric Li Date: Mon, 28 Mar 2022 15:39:02 +0000 Subject: [PATCH] [libTooling] Support TransformerResult in consumer callbacks Support `TransformerResult` 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 --- .../clang/Tooling/Transformer/Transformer.h | 101 ++++++++++-------- clang/lib/Tooling/Transformer/Transformer.cpp | 20 ---- 2 files changed, 58 insertions(+), 63 deletions(-) diff --git a/clang/include/clang/Tooling/Transformer/Transformer.h b/clang/include/clang/Tooling/Transformer/Transformer.h index db2feac06d4a..4e93783f2b9d 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 as well as the corresponding consumers. +/// \c RewriteRule as well as the corresponding consumers. class TransformerImpl { public: virtual ~TransformerImpl() = default; @@ -43,33 +43,6 @@ private: 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>)> Consumer; - -public: - explicit NoMetadataImpl( - transformer::RewriteRule R, - std::function>)> - 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 - buildMatchers() const final { - return transformer::detail::buildMatchers(Rule); - } -}; - // FIXME: Use std::type_identity or backport when available. template struct type_identity { using type = T; @@ -81,8 +54,6 @@ template struct TransformerResult { T Metadata; }; -// Specialization provided only to avoid SFINAE on the Transformer -// constructor; not intended for use. template <> struct TransformerResult { llvm::MutableArrayRef Changes; }; @@ -107,8 +78,14 @@ public: /// other. explicit Transformer(transformer::RewriteRuleWith Rule, ChangeSetConsumer Consumer) - : Impl(std::make_unique(std::move(Rule), - std::move(Consumer))) {} + : Transformer(std::move(Rule), + [Consumer = std::move(Consumer)]( + llvm::Expected> 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 @@ private: }; namespace detail { +/// Asserts that all \c Metadata for the \c Rule is set. +/// FIXME: Use constexpr-if in C++17. +/// @{ +template +void assertMetadataSet(const transformer::RewriteRuleWith &Rule) { + assert(llvm::all_of(Rule.Metadata, + [](const typename transformer::Generator &Metadata) + -> bool { return !!Metadata; }) && + "metadata generator must be provided for each rule"); +} +template <> +inline void assertMetadataSet(const transformer::RewriteRuleWith &) {} +/// @} + +/// Runs the metadata generator on \c Rule and stuffs it into \c Result. +/// FIXME: Use constexpr-if in C++17. +/// @{ +template +llvm::Error +populateMetadata(const transformer::RewriteRuleWith &Rule, + size_t SelectedCase, + const ast_matchers::MatchFinder::MatchResult &Match, + TransformerResult &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 &, size_t, + const ast_matchers::MatchFinder::MatchResult &Match, + TransformerResult &) { + return llvm::Error::success(); +} +/// @} + /// Implementation when metadata is generated as a part of the rewrite. This /// happens when we have a \c RewriteRuleWith. template class WithMetadataImpl final : public TransformerImpl { @@ -150,10 +165,7 @@ public: [](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 &Metadata) - -> bool { return !!Metadata; }) && - "metadata generator must be provided for each rule"); + assertMetadataSet(Rule); } private: @@ -174,16 +186,19 @@ private: Consumer(C.takeError()); return; } - } - - auto Metadata = Rule.Metadata[I]->eval(Result); - if (!Metadata) { - Consumer(Metadata.takeError()); + } else if (std::is_void::value) { + // If we don't have metadata and we don't have any edits, skip. return; } - Consumer(TransformerResult{llvm::MutableArrayRef(Changes), - std::move(*Metadata)}); + TransformerResult RewriteResult; + if (auto E = populateMetadata(Rule, I, Result, RewriteResult)) { + Consumer(std::move(E)); + return; + } + + RewriteResult.Changes = llvm::MutableArrayRef(Changes); + Consumer(std::move(RewriteResult)); } std::vector diff --git a/clang/lib/Tooling/Transformer/Transformer.cpp b/clang/lib/Tooling/Transformer/Transformer.cpp index 1cf824d2cbe4..f95f2ab7d954 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(*Changes)); -} - } // namespace detail void Transformer::registerMatchers(MatchFinder *MatchFinder) {