[PATCH] D82226: Add Metadata to Transformer tooling

ymandel added a comment. Looks good! Only real question is one of design -- should we consider the (deeper) change of templating the various types rather than using dynamic typing? For that matter, the decision doesn't even have to be the same for both AtomicChange and the Transformer types.

[PATCH] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

ymandel added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:335 + template struct is_same_method_impl { +static bool isSameMethod(...) { return false; } + }; Why use var-args rather than spelling out the type arguments like

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:16 +template +class RecordingVisitorBase : public TestVisitor { + bool VisitPostOrder;

[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.

This revision was automatically updated to reflect the committed changes. Closed by commit rG87340a2bf1d2: [libTooling] Delete deprecated `Stencil` combinators. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82225/new/

[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.

ymandel updated this revision to Diff 272157. ymandel added a comment. fix additional refs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82225/new/ https://reviews.llvm.org/D82225 Files: clang/include/clang/Tooling/Transformer/Stencil.h

[PATCH] D82126: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd81d69f1c0c1: [libTooling] Change Transformers `cat` to handle some cases of text in macros. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.

ymandel created this revision. ymandel added a reviewer: tdl-g. Herald added a project: clang. Deletes `text()` and `selection()` combinators, since they have been deprecated for months. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82225 Files:

[PATCH] D82179: Move TestClangConfig into libClangTesting and use it in AST Matchers tests

ymandel accepted this revision. ymandel marked an inline comment as done. ymandel added a comment. This revision is now accepted and ready to land. Only nits. Really nice work. I much prefer your new system, having wrestled with config and multi-language testing in the existing framework.

[PATCH] D82126: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

ymandel updated this revision to Diff 272150. ymandel added a comment. Fixed clang tidy lit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82126/new/ https://reviews.llvm.org/D82126 Files:

[PATCH] D82126: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

ymandel added a comment. In D82126#2104088 , @tdl-g wrote: > Interesting, in all three of those cases, it's reasonable to replace the > entire expression, thus eliminating the macro. None of those "tear" the > macro; if we had a case like > > #define

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel closed this revision. ymandel added a comment. Committed as revision rG9ca50e887db7 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81868/new/

[PATCH] D82126: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

ymandel added a comment. Tests show that this breaks the test for the clang tidy `abseil-string-find-str-contains`. Curious if this is a desirable change in behavior (in which case I'll update your test file) or the wrong behavior:

[PATCH] D82126: [libTooling] Change Transformer's `cat` to handle some cases of text in macros.

ymandel added a comment. In D82126#2103772 , @tdl-g wrote: > LGTM. I found the change description confusing, since it talks about the > selection() stencil but the code is all about the cat() stencil. I realize > (now) that the former is deprecated in

[PATCH] D82126: [libTooling] Change `selection` stencil to handle some cases of text in macros.

ymandel created this revision. ymandel added a reviewer: gribozavr2. Herald added a project: clang. Currently, the `selection` stencil validates the selection before extracting the text. However, this means that any range inside a macro is rejected as an error. This patch changes the

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel added a comment. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81868/new/ https://reviews.llvm.org/D81868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel updated this revision to Diff 271782. ymandel marked 3 inline comments as done. ymandel added a comment. removed Twine argument per clang-tidy warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81868/new/

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel updated this revision to Diff 271770. ymandel marked 3 inline comments as done. ymandel added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81868/new/ https://reviews.llvm.org/D81868 Files:

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel updated this revision to Diff 271246. ymandel added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81868/new/ https://reviews.llvm.org/D81868 Files: clang/include/clang/Tooling/Transformer/Parsing.h

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel marked 6 inline comments as done. ymandel added a comment. Thanks for the review! Comment at: clang/lib/Tooling/Transformer/Parsing.cpp:29 + +namespace { +using llvm::Error; gribozavr2 wrote: > I'm a bit concerned about the abundance of parsers in

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel updated this revision to Diff 270866. ymandel added a comment. Herald added a subscriber: mgorny. Added cmake changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81868/new/ https://reviews.llvm.org/D81868 Files:

[PATCH] D81868: [libTooling] Add parser for string representation of `RangeSelector`.

ymandel created this revision. ymandel added reviewers: gribozavr2, tdl-g. Herald added a project: clang. This patch adds a parser for a `RangeSelector` written as a string. The format is closely based on the way one would right the selector in C++, except that bound nodes are referred to by

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

ymandel added a comment. Thank you for bringing up this issue. I think it highlights an underlying problem -- editing templates is quite difficult -- that neither setting will address, as Dmitri expanded on above. Given the parallel to macros, I'd say your change is better than the status quo.

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

ymandel added a comment. Thanks for the changes! In D80697#2062517 , @njames93 wrote: > There are a few reasons for using the virtual method: I think I'm missing something (or I didn't explain my intention well): > - It keeps everything contained in

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

ymandel added a comment. Thanks for the suggestioned change. However, I think we can do somewhat simpler. What do you think of just providing a setter on the super class void setRule(Optional Rule) { this->Rule = std::move(Rule); } Subclasses will call `setRule` in their constructor body .

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

ymandel added a comment. Fixed with commit a1b88afe46d7a0f72d2acd8792951bd959b27545 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80023/new/ https://reviews.llvm.org/D80023

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7cfdff7b4a67: [clang-tidy] Add abseil-string-find-str-contains checker. (authored by tdl-g, committed by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

ymandel added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:286 + + virtual llvm::Optional TraversalKind() const { +return {}; aaron.ballman wrote: > `traversalKind()` Stephen -- What was the resolution on this comment? I

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

This revision was automatically updated to reflect the committed changes. Closed by commit rGce5780b88c6e: [libTooling] Fix Transformer to work with ambient traversal kinds. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

This revision was automatically updated to reflect the committed changes. ymandel marked an inline comment as done. Closed by commit rG04a96aa3e430: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher` (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

ymandel updated this revision to Diff 266837. ymandel added a comment. more typo fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80606/new/ https://reviews.llvm.org/D80606 Files: clang/include/clang/Tooling/Transformer/RewriteRule.h

[PATCH] D80704: Remove WrapperMatcherInterface

ymandel accepted this revision. ymandel added a comment. This revision is now accepted and ready to land. Yes, thank you! There are (IMO) too many layers of abstractions in this framework; this is a good step in the right direction. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

ymandel marked 3 inline comments as done. ymandel added inline comments. Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:154 +const RewriteRule , +ast_type_traits::TraversalKind DefaultTraversalKind) { // Map the cases into buckets of matchers -- one for

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

ymandel updated this revision to Diff 266835. ymandel marked 2 inline comments as done. ymandel added a comment. typo fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80606/new/ https://reviews.llvm.org/D80606 Files:

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

ymandel updated this revision to Diff 266831. ymandel added a comment. updated per comments and after rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80606/new/ https://reviews.llvm.org/D80606 Files:

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:234 +DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher InnerMatcher, +

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

ymandel updated this revision to Diff 266828. ymandel marked 2 inline comments as done. ymandel added a comment. Herald added a subscriber: mgorny. updated per comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80685/new/

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

ymandel updated this revision to Diff 266729. ymandel added a comment. adds a fix to transformer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80606/new/ https://reviews.llvm.org/D80606 Files:

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

ymandel created this revision. ymandel added reviewers: gribozavr, steveire. Herald added a project: clang. This patch exposes `TraversalKind` support in the `DynTypedMatcher` API. While previously, the `match` method supported traversal logic, it was not possible to set or get the traversal

[PATCH] D80654: WIP: Make it possible to use the traverse() matcher in clang-query

ymandel added a comment. Thanks, this looks quite useful. I should be able to look it over in full tomorrow. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:383 + static DynTypedMatcher + constructTraversalWrapper(const DynTypedMatcher , +

[PATCH] D80606: [libTooling][NFC] Demo bug introduced in D72534.

ymandel added a comment. In D80606#2057136 , @steveire wrote: > It might make sense for now (in order to unblock you) to make the > `Transformer` library explicitly require the `AsIs` mode. I am not so > familiar with the transformer library, but I

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

ymandel added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7018 +AST_MATCHER_P(clang::ParmVarDecl, isAtPosition, unsigned, N) { + return parmVarDecl(hasAncestor(functionDecl( + hasParameter(N,

[PATCH] D80606: [libTooling][NFC] Demo bug introduced in D72534.

ymandel created this revision. ymandel added reviewers: hokein, gribozavr. Herald added a project: clang. DO NOT PUSH. This patch includes two new tests that demo a bug introduced into Transformer by https://reviews.llvm.org/D72534. This patch is intended only as a demonstration of the problem.

[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

This revision was automatically updated to reflect the committed changes. Closed by commit rGff2743bf047d: [libTooling] In Transformer, allow atomic changes to span multiple files. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

ymandel updated this revision to Diff 266191. ymandel added a comment. fixes rebase mistake in previous diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80239/new/ https://reviews.llvm.org/D80239 Files:

[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

ymandel updated this revision to Diff 266190. ymandel marked an inline comment as done. ymandel added a comment. Herald added subscribers: llvm-commits, dmgreen. Herald added a project: LLVM. adds fixme comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

ymandel marked 2 inline comments as done. ymandel added a comment. Thanks for the review. Comment at: clang/lib/Tooling/Transformer/Transformer.cpp:65 - for (const auto : Case.AddedIncludes) { -auto = I.first; -switch (I.second) { -case

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

ymandel added a comment. LGTM, but leaving for one of the other reviewers to give you the official "Accept Revision". Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:72 + binaryOperator(hasOperatorName("=="), +

[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added subscribers: jfb, mgrang. Herald added a project: clang. Currently, all changes returned by a single application of a rule must fit in one atomic change and therefore must apply to one file. However, there are

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

ymandel added a comment. Looks good, just some nits! Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:48 + Options.get("StringLikeClasses", DefaultStringLikeClasses)); + const std::string AbseilStringsMatchHeader( +

[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option IncludeStyle.

This revision was automatically updated to reflect the committed changes. Closed by commit rGc5b1a0352535: [clang-tidy] In TransformerClangTidyCheck, support option IncludeStyle. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option SourceNamingStyle.

ymandel updated this revision to Diff 262105. ymandel added a comment. renamed option to IncludeStyle, for consistency w/ existing checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79380/new/ https://reviews.llvm.org/D79380 Files:

[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option SourceNamingStyle.

ymandel added a comment. In D79380#2020135 , @gribozavr2 wrote: > > It's only effect is to determine how a given file is related to header > > files, specifically how to determine that a header "corresponds" to a the > > file being examined -- that is,

[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option SourceNamingStyle.

ymandel added a comment. In D79380#2019570 , @gribozavr2 wrote: > LGTM, but I don't understand why this option is called "SourceNamingStyle". > Existing ClangTidy checkers call it "IncludeStyle". Thanks for the review! Good question. I'd planned to

[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option SourceNamingStyle.

ymandel created this revision. ymandel added a reviewer: gribozavr2. Herald added a subscriber: xazax.hun. Herald added a project: clang. The new option allows the user to specify which file naming convention is used by the source code ('llvm' or 'google'). Repository: rG LLVM Github Monorepo

[PATCH] D77419: [libTooling] Simplify the representation of Transformer's RewriteRules.

This revision was automatically updated to reflect the committed changes. Closed by commit rG5e5d36671833: [libTooling] Simplify the representation of Transformers RewriteRules. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77419: [libTooling] Simplify the representation of Transformer's RewriteRules.

ymandel marked 2 inline comments as done. ymandel added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77419/new/ https://reviews.llvm.org/D77419 ___ cfe-commits

[PATCH] D77419: [libTooling] Simplify the representation of Transformer's RewriteRules.

ymandel updated this revision to Diff 255734. ymandel added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77419/new/ https://reviews.llvm.org/D77419 Files: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp

[PATCH] D77419: [libTooling] Simplify the representation of Transformer's RewriteRules.

ymandel updated this revision to Diff 255733. ymandel added a comment. replaced name; fixed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77419/new/ https://reviews.llvm.org/D77419 Files:

[PATCH] D77419: [libTooling] Simplify the representation of Transformer's RewriteRules.

ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:40 + +/// A map from a match result to a list of concrete errors (with possible +/// failure). This type is a building block of rewrite

[PATCH] D77419: [libTooling] Simplify the representation of Transformer's RewriteRules.

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

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

ymandel accepted this revision. ymandel added a comment. This revision is now accepted and ready to land. Thanks for the example and, generally, explanation. Just a few small comments. Comment at:

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

ymandel added a comment. Thanks for expanding the description, including the helpful example. I'm not sure, though, that this is the "right" behavior, at least not always. Worse, I'm not sure there is a single "right" behavior. I can easily imagine a tidy that matches multiple times in the

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

ymandel added a comment. Can you please expand on the description? Specifically, can you clarify what you've *changed* in this patch? I gather that previously, it distinguished between two different sources of replacements: diagnostics and otherwise, and only deduplicated the ones from

[PATCH] D75556: [AST Matchers] Restrict `optionally` matcher to a single argument.

This revision was automatically updated to reflect the committed changes. Closed by commit rGc359f9537ffb: [AST Matchers] Restrict `optionally` matcher to a single argument. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75556: [AST Matchers] Restrict `optionally` matcher to a single argument.

ymandel updated this revision to Diff 248170. ymandel added a comment. Updated comments for matcher. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75556/new/ https://reviews.llvm.org/D75556 Files: clang/include/clang/ASTMatchers/ASTMatchers.h

[PATCH] D75556: [AST Matchers] Restrict `optionally` matcher to a single argument.

ymandel updated this revision to Diff 248171. ymandel added a comment. Updated the matcher HTML doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75556/new/ https://reviews.llvm.org/D75556 Files: clang/docs/LibASTMatchersReference.html

[PATCH] D75556: [AST Matchers] Restrict `optionally` matcher to a single argument.

ymandel created this revision. ymandel added reviewers: sbenza, aaron.ballman. Herald added a project: clang. Currently, `optionally` can take multiple arguments, which commits it to a particular strategy for those arguments (in this case, "for each"). We limit the matcher to a single argument,

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

ymandel added a comment. In D75365#1900784 , @gribozavr2 wrote: > +1 to this fix. > > However, regarding `allOf` vs. `anyOf` semantics, since `optionally` always > succeeds, is there a difference between the two semantics? > > It seems to me that there

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

This revision was automatically updated to reflect the committed changes. Closed by commit rG586f13aeac3f: [AST Matchers] Fix bug in optionally matcher wherein all previous bindings… (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

ymandel updated this revision to Diff 247314. ymandel added a comment. fix formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75365/new/ https://reviews.llvm.org/D75365 Files: clang/lib/ASTMatchers/ASTMatchersInternal.cpp

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

ymandel added a comment. In D75365#1898536 , @aaron.ballman wrote: > ... > Good question! My intuition is that `optionally` should take exactly one > argument and the user should be explicit as to whether they mean `allOf` or > `anyOf` when there is

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

ymandel added a comment. Aaron -- when fixing this bug, I had some other questions about the behavior of this matcher. Specifically, instead of adding bindings to the existing map, it creates a new match result for each matching node (with addMatch). But, I'm struggling to see how it makes

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

ymandel created this revision. ymandel added reviewers: aaron.ballman, sbenza. Herald added a project: clang. ymandel updated this revision to Diff 247299. ymandel added a comment. removed change to unrelated file. The implementation of 'optionally' doesn't preserve bindings when none of the

[PATCH] D75365: [AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

ymandel updated this revision to Diff 247299. ymandel added a comment. removed change to unrelated file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75365/new/ https://reviews.llvm.org/D75365 Files:

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rG38b4516de8a4: [libTooling] Add function to determine associated text of a declaration. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel updated this revision to Diff 246706. ymandel added a comment. Fix failing test under msvc compatibility. Adds -fno-delayed-template-parsing to compilation arguments in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel reopened this revision. ymandel added a comment. This revision is now accepted and ready to land. Reopening for fix to failing tests that resulted in revert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9c54f6154f74: [libTooling] Add function to determine associated text of a declaration. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel added a comment. Thanks for the detailed review, especially given the complexity of the code! Comment at: clang/lib/Tooling/Transformer/SourceCode.cpp:152 +// will not break anything that removing the entity wouldn't have +// already broken. + bool

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel updated this revision to Diff 246556. ymandel marked 8 inline comments as done. ymandel added a comment. Responded to all comments. No functional changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/

[PATCH] D74840: [AST matchers] Add basic matchers for googletest EXPECT/ASSERT calls.

This revision was automatically updated to reflect the committed changes. Closed by commit rG23444edf30ba: [AST matchers] Add basic matchers for googletest EXPECT/ASSERT calls. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74840: [AST matchers] Add basic matchers for googletest EXPECT/ASSERT calls.

ymandel created this revision. ymandel added a reviewer: gribozavr2. Herald added a subscriber: mgorny. Herald added a project: clang. This revision adds matchers that match calls to the gtest EXPECT and ASSERT macros almost like function calls. The matchers are placed in separate files

[PATCH] D74763: [libTooling] Add option for `buildAST` to report diagnostics.

This revision was automatically updated to reflect the committed changes. Closed by commit rG523cae324d79: [libTooling] Add option for `buildAST` to report diagnostics. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74763/new/

[PATCH] D74763: [libTooling] Add option for `buildAST` to report diagnostics.

ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. Currently, `buildAST[WithArgs]` either succeeds or fails. This patch adds support for the caller to pass a `DiagnosticConsumer` to receive all relevant diagnostics. Repository: rG LLVM Github

[PATCH] D74214: [clang-tidy] Fix PR#34798 'readability-braces-around-statements breaks statements containing braces.'

ymandel added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp:76-77 + + // We need to check that it is not 'InitListExpr' which ends with + // the tokens '};' because it will break the following analysis + tok::TokenKind

[PATCH] D74214: [clang-tidy] Fix PR#34798 'readability-braces-around-statements breaks statements containing braces.'

ymandel added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp:21 -tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager , -const ASTContext *Context) { - Token Tok; +bool

[PATCH] D73975: [clang][NFC] Expand some `auto`s and add another test for matcher `isExpandedFromMacro`.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe5ff92e049b5: [clang][NFC] Expand some `auto`s and add another test for matcher… (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73975: [clang][NFC] Expand some `auto`s and add another test for matcher `isExpandedFromMacro`.

ymandel created this revision. ymandel added a reviewer: aaron.ballman. Herald added a project: clang. Spells out some `auto`s explicitly and adds another test for the matcher `isExpandedFromMacro`. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D73975 Files:

[PATCH] D73965: [clang] Add matcher to identify macro expansions.

ymandel added a comment. Aaron -- I missed your comments before submitting. I'll send a follow up with fixes. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73965/new/ https://reviews.llvm.org/D73965

[PATCH] D73965: [clang] Add matcher to identify macro expansions.

This revision was automatically updated to reflect the committed changes. Closed by commit rG386fd2c170a7: [clang] Add matcher to identify macro expansions. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73965/new/

[PATCH] D73965: [clang] Add matcher to identify macro expansions.

ymandel updated this revision to Diff 242335. ymandel added a comment. Fix to get it to link correctly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73965/new/ https://reviews.llvm.org/D73965 Files:

[PATCH] D73965: [clang] Add matcher to identify macro expansions.

ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. This revision adds a matcher `isExpandedFromMacro` that determines whether a statement is (transitively) expanded from a given macro. Repository: rG LLVM Github Monorepo

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel marked an inline comment as done. ymandel added a comment. Thank you for the detailed review. I've significantly expanded and refactored the tests. I also lifted `validateEditRange` into its own function and added corresponding tests. Comment at:

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel updated this revision to Diff 241127. ymandel marked 12 inline comments as done. ymandel added a comment. tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/ https://reviews.llvm.org/D72153 Files:

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel updated this revision to Diff 241124. ymandel marked 4 inline comments as done. ymandel added a comment. addressed comments, with significant reworking of tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/

[PATCH] D72274: [libTooling] Fix bug in Stencil handling of macro ranges

This revision was automatically updated to reflect the committed changes. Closed by commit rGb9d2bf38e86e: [libTooling] Fix bug in Stencil handling of macro ranges (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72274/new/

[PATCH] D72274: [libTooling] Fix bug in Stencil handling of macro ranges

ymandel marked an inline comment as done. ymandel added a comment. Thanks for the review! In D72274#1826634 , @gribozavr2 wrote: > In D72274#1826614 , @ymandel wrote: > > > In D72274#1826477

[PATCH] D72274: [libTooling] Fix bug in Stencil handling of macro ranges

ymandel marked 2 inline comments as done. ymandel added a comment. In D72274#1826477 , @gribozavr2 wrote: > The only functional change that I see in this patch is in > `clang/lib/Tooling/Transformer/Stencil.cpp`. However, I don't understand how > that

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel updated this revision to Diff 237051. ymandel added a comment. fold initLexer into the callsite, because it's only called once. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/ https://reviews.llvm.org/D72153 Files:

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

ymandel updated this revision to Diff 237044. ymandel added a comment. tweaked test organization Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/ https://reviews.llvm.org/D72153 Files:

<    5   6   7   8   9   10   11   12   13   14   >