ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added a subscriber: jfb.
Herald added a project: clang.
This revision simplifies the representation of edits in rewrite rules. The
simplified form is more general, allowing the user more flexibility in building
custom edit specifications.
The changes extend the API, without changing the signature of existing
functions. So this only risks breaking users that directly accessed the
`RewriteRule` struct.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77419
Files:
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/lib/Tooling/Transformer/Transformer.cpp
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===================================================================
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -31,7 +31,7 @@
transformer::RewriteRule::Case Case =
transformer::detail::findSelectedCase(Result, Rule);
- auto Transformations = transformer::detail::translateEdits(Result, Case.Edits);
+ auto Transformations = Case.Edits(Result);
if (!Transformations) {
Consumer(Transformations.takeError());
return;
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===================================================================
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -28,12 +28,11 @@
using MatchResult = MatchFinder::MatchResult;
-Expected<SmallVector<transformer::detail::Transformation, 1>>
-transformer::detail::translateEdits(const MatchResult &Result,
- llvm::ArrayRef<ASTEdit> Edits) {
- SmallVector<transformer::detail::Transformation, 1> Transformations;
- for (const auto &Edit : Edits) {
- Expected<CharSourceRange> Range = Edit.TargetRange(Result);
+static Expected<SmallVector<transformer::Edit, 1>>
+translateEdits(const MatchResult &Result, ArrayRef<ASTEdit> ASTEdits) {
+ SmallVector<transformer::Edit, 1> Edits;
+ for (const auto &E : ASTEdits) {
+ Expected<CharSourceRange> Range = E.TargetRange(Result);
if (!Range)
return Range.takeError();
llvm::Optional<CharSourceRange> EditRange =
@@ -41,21 +40,33 @@
// FIXME: let user specify whether to treat this case as an error or ignore
// it as is currently done.
if (!EditRange)
- return SmallVector<Transformation, 0>();
- auto Replacement = Edit.Replacement->eval(Result);
+ return SmallVector<Edit, 0>();
+ auto Replacement = E.Replacement->eval(Result);
if (!Replacement)
return Replacement.takeError();
- transformer::detail::Transformation T;
+ transformer::Edit T;
T.Range = *EditRange;
T.Replacement = std::move(*Replacement);
- Transformations.push_back(std::move(T));
+ Edits.push_back(std::move(T));
}
- return Transformations;
+ return Edits;
}
-ASTEdit transformer::changeTo(RangeSelector S, TextGenerator Replacement) {
+EditList transformer::editList(SmallVector<ASTEdit, 1> Edits) {
+ return [Edits = std::move(Edits)](const MatchResult &Result) {
+ return translateEdits(Result, Edits);
+ };
+}
+
+EditList transformer::edit(ASTEdit Edit) {
+ return [Edit = std::move(Edit)](const MatchResult &Result) {
+ return translateEdits(Result, {Edit});
+ };
+}
+
+ASTEdit transformer::changeTo(RangeSelector Target, TextGenerator Replacement) {
ASTEdit E;
- E.TargetRange = std::move(S);
+ E.TargetRange = std::move(Target);
E.Replacement = std::move(Replacement);
return E;
}
@@ -82,8 +93,8 @@
return change(std::move(S), std::make_shared<SimpleTextGenerator>(""));
}
-RewriteRule transformer::makeRule(DynTypedMatcher M, SmallVector<ASTEdit, 1> Edits,
- TextGenerator Explanation) {
+RewriteRule transformer::makeRule(ast_matchers::internal::DynTypedMatcher M,
+ EditList Edits, TextGenerator Explanation) {
return RewriteRule{{RewriteRule::Case{
std::move(M), std::move(Edits), std::move(Explanation), {}}}};
}
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===================================================================
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -30,6 +30,19 @@
namespace clang {
namespace transformer {
+/// A concrete description of a source edit, represented by a character range in
+/// the source to be replaced and a corresponding replacement string.
+struct Edit {
+ CharSourceRange Range;
+ std::string Replacement;
+};
+
+/// A map from a match result to a list of concrete errors (with possible
+/// failure). This type is a building block of rewrite rules, but users will
+/// generally work in terms of `ASTEdit`s (below) rather than directly in terms
+/// of `EditList`.
+using EditList = MatchConsumer<llvm::SmallVector<Edit, 1>>;
+
using TextGenerator = std::shared_ptr<MatchComputation<std::string>>;
// Description of a source-code edit, expressed in terms of an AST node.
@@ -74,6 +87,19 @@
TextGenerator Note;
};
+/// Lifts a list of `ASTEdit`s into an `EditList`.
+///
+/// The `EditList` will return an empty vector if any of the edits apply to
+/// portions of the source that are ineligible for rewriting (certain
+/// interactions with macros, for example) and it will fail if any invariants
+/// are violated relating to bound nodes in the match. However, it does not
+/// fail in the case of conflicting edits -- conflict handling is left to
+/// clients. We recommend use of the \c AtomicChange or \c Replacements classes
+/// for assistance in detecting such conflicts.
+EditList editList(llvm::SmallVector<ASTEdit, 1> Edits);
+// Convenience form of `editList` for a single edit.
+EditList edit(ASTEdit);
+
/// Format of the path in an include directive -- angle brackets or quotes.
enum class IncludeFormat {
Quoted,
@@ -106,7 +132,7 @@
struct RewriteRule {
struct Case {
ast_matchers::internal::DynTypedMatcher Matcher;
- SmallVector<ASTEdit, 1> Edits;
+ EditList Edits;
TextGenerator Explanation;
// Include paths to add to the file affected by this case. These are
// bundled with the `Case`, rather than the `RewriteRule`, because each case
@@ -122,17 +148,23 @@
};
/// Convenience function for constructing a simple \c RewriteRule.
-RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
- SmallVector<ASTEdit, 1> Edits,
+RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M, EditList Edits,
TextGenerator Explanation = nullptr);
+/// Convenience function for constructing a \c RewriteRule from multiple
+/// `ASTEdit`s.
+inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
+ llvm::SmallVector<ASTEdit, 1> Edits,
+ TextGenerator Explanation = nullptr) {
+ return makeRule(std::move(M), editList(std::move(Edits)),
+ std::move(Explanation));
+}
+
/// Convenience overload of \c makeRule for common case of only one edit.
inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
ASTEdit Edit,
TextGenerator Explanation = nullptr) {
- SmallVector<ASTEdit, 1> Edits;
- Edits.emplace_back(std::move(Edit));
- return makeRule(std::move(M), std::move(Edits), std::move(Explanation));
+ return makeRule(std::move(M), edit(std::move(Edit)), std::move(Explanation));
}
/// For every case in Rule, adds an include directive for the given header. The
@@ -260,28 +292,6 @@
const RewriteRule::Case &
findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
const RewriteRule &Rule);
-
-/// A source "transformation," represented by a character range in the source to
-/// be replaced and a corresponding replacement string.
-struct Transformation {
- CharSourceRange Range;
- std::string Replacement;
-};
-
-/// Attempts to translate `Edits`, which are in terms of AST nodes bound in the
-/// match `Result`, into Transformations, which are in terms of the source code
-/// text.
-///
-/// Returns an empty vector if any of the edits apply to portions of the source
-/// that are ineligible for rewriting (certain interactions with macros, for
-/// example). Fails if any invariants are violated relating to bound nodes in
-/// the match. However, it does not fail in the case of conflicting edits --
-/// conflict handling is left to clients. We recommend use of the \c
-/// AtomicChange or \c Replacements classes for assistance in detecting such
-/// conflicts.
-Expected<SmallVector<Transformation, 1>>
-translateEdits(const ast_matchers::MatchFinder::MatchResult &Result,
- llvm::ArrayRef<ASTEdit> Edits);
} // namespace detail
} // namespace transformer
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -73,16 +73,15 @@
assert(Rule && "check() should not fire if Rule is None");
RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, *Rule);
- Expected<SmallVector<transformer::detail::Transformation, 1>>
- Transformations = transformer::detail::translateEdits(Result, Case.Edits);
- if (!Transformations) {
- llvm::errs() << "Rewrite failed: "
- << llvm::toString(Transformations.takeError()) << "\n";
+ Expected<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
+ if (!Edits) {
+ llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
+ << "\n";
return;
}
// No rewrite applied, but no error encountered either.
- if (Transformations->empty())
+ if (Edits->empty())
return;
Expected<std::string> Explanation = Case.Explanation->eval(Result);
@@ -93,9 +92,8 @@
}
// Associate the diagnostic with the location of the first change.
- DiagnosticBuilder Diag =
- diag((*Transformations)[0].Range.getBegin(), *Explanation);
- for (const auto &T : *Transformations)
+ DiagnosticBuilder Diag = diag((*Edits)[0].Range.getBegin(), *Explanation);
+ for (const auto &T : *Edits)
Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
for (const auto &I : Case.AddedIncludes) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
