[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-13 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. ymandel marked an inline comment as done. Closed by commit rL368681: [libTooling] In Transformer, generalize `applyFirst` to admit rules with… (authored by ymandel, committed by ). Herald added a project: LLVM. Herald added

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127 +if (!hasValidKind(Cases[I].Matcher)) + return std::vector(); +Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I,

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 214700. ymandel added a comment. Changed early return to assertion; updated test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65877/new/ https://reviews.llvm.org/D65877 Files:

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127 +if (!hasValidKind(Cases[I].Matcher)) + return std::vector(); +Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]);

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added a comment. In D65877#1625616 , @gribozavr wrote: > I'd prefer we ban them completely, and let the user wrap them manually if > they need to. While some users would appreciate the ergonomics, I think

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 214679. ymandel added a comment. Bans qualtype and type and adds corresponding comments and test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65877/new/ https://reviews.llvm.org/D65877 Files:

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D65877#1625493 , @ymandel wrote: > I was going to add a test for `Type`/`QualType` and realized that they don't > carry any source location info. Therefore, I don't think they belong as > top-level matchers for rewrite

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added a comment. I was going to add a test for `Type`/`QualType` and realized that they don't carry any source location info. Therefore, I don't think they belong as top-level matchers for rewrite rules. Instead, users should use

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Nice simplification, thanks! Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127 + // in `taggedMatchers`. + std::map, 1>> + Buckets;

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added a comment. Thanks for your detailed and helpful feedback. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:129 + Buckets.back().Cases.emplace_back(CaseId, std::move(Case)); } gribozavr wrote:

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 214442. ymandel marked 14 inline comments as done. ymandel added a comment. Rewrote the code and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65877/new/ https://reviews.llvm.org/D65877 Files:

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:85 // the implicit relationship of Type and QualType. static bool isBaseOf(ASTNodeKind A, ASTNodeKind B) { static auto TypeKind = ASTNodeKind::getFromNodeKind(); I'd

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-07 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 213932. ymandel added a comment. fix comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65877/new/ https://reviews.llvm.org/D65877 Files: clang/include/clang/Tooling/Refactoring/Transformer.h

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-07 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. This patch removes an (artificial) limitation of `applyFirst`, which requires that all of the rules' matchers can be grouped together in a single `anyOf()`. This change generalizes the code to group