[PATCH] D125091: [clang][dataflow][NFC] Clarify guarantees on returned vector size for `runDataflowAnalysis`.

2022-05-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa45647d82b72: [clang][dataflow][NFC] Clarify guarantees on returned vector size for… (authored by ymandel). Repository: rG LLVM Github Monorepo C

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

2022-05-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:513 + const RecordDecl *RD = BaseTy->getDecl(); + if (RD->getIdentifier() == nullptr || RD->getName() != "Message") +return false; -

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

2022-05-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7e63a0d479dd: [clang-tidy] New check for safe usage of `std::optional` and like types. (authored by ymandel). Changed prior to commit: https://rev

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

2022-07-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Sam, this is a great start! I'm really excited to see that you have a core working so quickly. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:208 + + // TODO: Currently this only works if the callee is never a method and the + /

[PATCH] D130270: [clang][dataflow] Use a dedicated bool to encode which branch was taken

2022-07-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp:439 Context.addFlowConditionConstraint(FC2, Y); - // JoinedFC = (FC1 || FC2) && Z = (X || Y) && Z - auto &JoinedFC = Context.joinFlowConditions(FC1, FC2); + //

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

2022-07-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:208 + + // TODO: Currently this only works if the callee is never a method and the + // same callee is never analyzed from multiple separate callsites. To samest

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

2022-07-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:220 + assert(Body != nullptr); + initGlobalVars(*Body, Env); + samestep wrote: > ymandel wrote: > > samestep wrote: > > > ymandel wrote: > > > > I wonder how thi

[PATCH] D130398: [clang][dataflow] Add Environment::dump()

2022-07-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp:117-120 +for (const std::string &S : ConstraintsStrings) { + Result += S; + Result += '\n';

[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

2022-07-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D130306#3676325 , @samestep wrote: > >> The main reason I wanted to call this out because it increasingly seems to >> be whenever a decision needs to be made, the framework is getting less and >> less sound. Basically, ma

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

2022-05-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 8 inline comments as done. ymandel added a comment. I'm having trouble with arcanist -- `arc diff` is crashing, but wanted to respond in the meantime to your questions. I hope I'll be able to actually upload the new diff soon... Comment at: clang/lib/Analysis/

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

2022-05-13 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 429210. ymandel marked 6 inline comments as done. ymandel added a comment. address reviewer comments and fix bug in how the nested value as _re_created -- now uses the loc's type rather than the expression's type (which doesn't always match exactly). CHANG

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

2022-05-13 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 429211. ymandel added a comment. remove stray FIXME from previous diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124932/new/ https://reviews.llvm.org/D124932 Files: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp cl

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

2022-05-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:574 -// Sub-expressions that are logic operators are not added in basic blocks -// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic -// operator, it isn't e

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

2022-05-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125821/new/ https://reviews.llvm.org/D125821 __

[PATCH] D125931: [clang][dataflow] Add support for correlated branches to optional model

2022-05-19 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thanks! Looks good overall, just the one question about loops. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:596-598 + MergedEnv.makeOr( + MergedEnv.makeAnd(Env1.getFlowConditionToken(), *HasValueVal

[PATCH] D125931: [clang][dataflow] Add support for correlated branches to optional model

2022-05-20 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D125931#3525699 , @xazax.hun wrote: > Actually, I think in most cases we want to be consistent on how to merge bool > values. So I wonder whether instead of reimplementing the merge operation in > this check we should just ca

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

2022-05-24 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. sgatev@ -- gentle ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124932/new/ https://reviews.llvm.org/D124932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D126314: [clang][dataflow] Relax `Environment` comparison operation.

2022-05-24 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added reviewers: xazax.hun, sgatev, li.zhe.hua. Herald added subscribers: tschuett, steakhal, rnkovacs. Herald added a project: All. ymandel requested review of this revision. Herald added a project: clang. Ignore `MemberLocToStruct` in environment comparison

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

2019-08-23 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. Every change triggered by a rewrite rule is anchored at a particular location in the source code. This patch refines how that location is chosen when the rule matches within a macro and defines it a

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

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

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

2019-08-23 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 216921. ymandel marked 2 inline comments as done. ymandel added a comment. comments tweaks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66652/new/ https://reviews.llvm.org/D66652 Files: clang/include/clang

[PATCH] D66676: [clang-tidy] TransformerClangTidyCheck: change choice of location for diagnostic message.

2019-08-23 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a subscriber: xazax.hun. Herald added a project: clang. This patch changes the location specified to the `ClangTidyCheck::diag()`. Currently, the beginning of the matched range is used. This patch uses the beginning o

[PATCH] D66676: [clang-tidy] TransformerClangTidyCheck: change choice of location for diagnostic message.

2019-08-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 217146. ymandel added a comment. fixed comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66676/new/ https://reviews.llvm.org/D66676 Files: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cp

[PATCH] D66676: [clang-tidy] TransformerClangTidyCheck: change choice of location for diagnostic message.

2019-08-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp:106 +// argument, while the change spans only the argument AND there are two such +// matches. Here, we expect a conflic

[PATCH] D66676: [clang-tidy] TransformerClangTidyCheck: change choice of location for diagnostic message.

2019-08-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL369914: [clang-tidy] TransformerClangTidyCheck: change choice of location for… (authored by ymandel, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

2019-08-27 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. I'm having second thoughts about this -- I prefer the approach I ended up taking in https://reviews.llvm.org/D66676, which is subtly different. However, getRuleMatchLoc() will be useful a different purpose: when only reporting a diagnostic, with no corresponding changes

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a subscriber: xazax.hun. Herald added a project: clang. The bugprone-use-after-move check exhibits false positives for certain uses of the C++17 if/switch init statements. These false positives are caused by a bug in

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 219152. ymandel added a comment. Added tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67292/new/ https://reviews.llvm.org/D67292 Files: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp clang-tools

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-use-after-move.cpp:1198 +} + } for (int i = 0; i < 10; ++i) { gribozavr wrote: > Unless you think it is redundant, could you al

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-09 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 rL371396: [clang-tidy] Fix bug in bugprone-use-after-move check (authored by ymandel, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-c

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 3 inline comments as done. ymandel added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-use-after-move.cpp:1198 +} + } for (int i = 0; i < 10; ++i) { mboehme wrote: > gribozavr wrote: > > ymandel wrote: > > > griboz

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

2020-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. This patch adds `getAssociatedRange` which, for a given decl, computes preceding and trailing text that would conceptually be associated with the decl by the reader. This includes comments, whitespac

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

2020-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 236057. ymandel added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/ https://reviews.llvm.org/D72153 Files: clang/include/clang/Tooling/Transformer/SourceCode.h clang/lib/

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

2020-01-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. Currently, an attempt to rewrite source code inside a macro expansion succeeds, but results in empty text, rather than failing with an error. This patch restructures to the code to explicitly vali

[PATCH] D71842: Allow newlines in AST Matchers in clang-query files

2020-01-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/include/clang/ASTMatchers/Dynamic/Parser.h:167 static llvm::Optional - parseMatcherExpression(StringRef MatcherCode, Sema *S, - const NamedValueMap *NamedValues, - Diagnostics *Er

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

2020-01-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 236879. ymandel added a comment. Fixed behavior in some corner cases; added tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/ https://reviews.llvm.org/D72153 Files: clang/include/clang/Tooling/T

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

2020-01-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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: clang/include/clang/Tooling/Transformer/SourceCode.h

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

2020-01-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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: clang/include

[PATCH] D87409: [libTooling] Fix use of `char` in comparison.

2020-09-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa5cefd95cc60: [libTooling] Fix use of `char` in comparison. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87409/new/ https://reviews.

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

2020-09-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Can you expand on what is wrong currently for `FunctionDecl` descendants? Would the new test `FindsBodyOfFunctionChildren` fail with the current implementation? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87527/new/ http

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

2020-09-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D87527#2271016 , @baloghadamsoftware wrote: > In D87527#2270939 , @ymandel wrote: > >> Can you expand on what is wrong currently for `FunctionDecl` descendants? >> Would the new test `F

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

2020-09-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D87527#2271059 , @baloghadamsoftware wrote: > We must decide about the namings. If we want to be in sync with the methods > in `FunctionDecl`, then we keep `hasBody()` as is, but remove the template > specialization, and crea

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

2020-09-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added a comment. Thanks, this looks great! But, can you also please update https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp#L39, since it depends on the current semantics? CHANGES SINCE LAS

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-09-24 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr2. Herald added a project: clang. ymandel requested review of this revision. This matcher combines the features of the `hasParent` matcher with skip-implicit-expression-nodes behavior of `ignoringImplicit`. It finds the first ancest

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-09-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 294312. ymandel added a comment. update dynamic registry and the ast matcher doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88275/new/ https://reviews.llvm.org/D88275 Files: clang/docs/LibASTMatchersRefe

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-09-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 294348. ymandel added a comment. Fixed to use more standard type adaptors. Registration now works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88275/new/ https://reviews.llvm.org/D88275 Files: clang/docs/L

[PATCH] D88319: [AST] Delete broken, unused template.

2020-09-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr2. Herald added a project: clang. ymandel requested review of this revision. The `const*` overload of `IgnoreExprNodes` can't compile. Moreover, this didn't turn up because it's unused. There's no obvious fix, so I've deleted it.

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-09-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D88275#2295379 , @aaron.ballman wrote: > This seems to be strongly related to the `TK_IgnoreUnlessSpelledInSource` > traversal behavior; is there a reason you can't use that traversal mode with > `hasParent()` (does it behave

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-09-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 294363. ymandel added a comment. restored changes to unrelated parts of html docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88275/new/ https://reviews.llvm.org/D88275 Files: clang/docs/LibASTMatchersRef

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. ymandel requested review of this revision. The new overloads apply directly to a node, like the `clang::ast_matchers::match` functions, Rather than generating an `EditGenerator` combinator. Reposit

[PATCH] D87048: [libTooling] Restore defaults for matchers in makeRule.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr. Herald added a project: clang. ymandel requested review of this revision. This patch restores the default traversal for Transformer's `makeRule` to `TK_AsIs`. The implicit mode has proven problematic. Repository: rG LLVM Githu

[PATCH] D87048: [libTooling] Restore defaults for matchers in makeRule.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6f0a3711bc15: [libTooling] Restore defaults for matchers in makeRule. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87048/new/ https:

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thanks for the review! Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:388 +llvm::Expected> +rewriteDescendants(const Decl &Node, RewriteRule Rule, + const ast_matchers::MatchFinder::MatchResult &Result);

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 289623. ymandel added a comment. Herald added a subscriber: JDevlieghere. fix diff base; make small clang-tidy suggested change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87031/new/ https://reviews.llvm.org/

[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd4f390313129: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on… (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D87409: [libTooling] Fix use of `char` in comparison.

2020-09-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: lbenes. Herald added a project: clang. ymandel requested review of this revision. Fixes Transformer's `Range` parser to handle `char` in a platform-independent way. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D87409 F

[PATCH] D93637: [libTooling] Add support for smart pointers to releveant Transformer `Stencil`s.

2020-12-21 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added reviewers: tdl-g, sbenza. ymandel requested review of this revision. Herald added a project: clang. Stencils `maybeDeref` and `maybeAddressOf` are designed to handle nodes that may be pointers. Currently, they only handle native pointers. This patch ext

[PATCH] D93695: [clang-tidy] Update uses of deprecated Transformer APIs in StringFindStrContainsCheck.

2020-12-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: tdl-g. Herald added a subscriber: xazax.hun. ymandel requested review of this revision. Herald added a project: clang. Migrates `change` to `changeTo`; changes to new constructor API (2-arg construct + `setRule`); refactors use of `addInclud

[PATCH] D93703: [libTooling] Change `addInclude` to use expansion locs.

2020-12-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: tdl-g. ymandel requested review of this revision. Herald added a project: clang. This patch changes the default range used to anchor the include insertion to use an expansion loc. This ensures that the location is valid, when the user relie

[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

2020-11-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:29-33 +static ast_matchers::internal::Matcher +hasAnyNameStdString(std::vector Names) { + return ast_matchers::internal::Matcher( + new ast_matchers::internal::HasNa

[PATCH] D91544: [clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule directly.

2020-11-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: gribozavr2. Herald added a subscriber: xazax.hun. Herald added a project: clang. ymandel requested review of this revision. Adds support for setting the `Rule` field. In the process, refactors the code that accesses that field and adds a co

[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

2020-11-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. LGTM. I'll leave it to Aaron, though, to accept. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91015/new/ https://reviews.llvm.org/D91015 ___ cfe-commits mailing list cfe-commits

[PATCH] D91544: [clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule directly.

2020-11-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG068da2c749a5: [clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule directly. (authored by ymandel). Changed prior to commit: https://reviews.llvm.org/D91544?vs=305516&id=306151#toc Re

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-09-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. > I'm not concerned about the basic idea behind the proposed matcher, I'm only > worried we're making AST matching more confusing by having two different ways > of inconsistently accomplishing the same-ish thing. Aaron, I appreciate this concern, but I would argue that

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-10-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D88275#2305989 , @aaron.ballman wrote: > In D88275#2303283 , @ymandel wrote: > >>> I'm not concerned about the basic idea behind the proposed matcher, I'm >>> only worried we're making

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-10-07 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D88275#2316484 , @aaron.ballman wrote: > Thank you for your patience while we sort this out. I've pinged @steveire > off-list, so hopefully he'll respond with his thoughts. If we don't hear from > Stephen in the next week or

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

2020-10-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. TL;DR Stephen's fix works; I'll drop this patch. > Why do you use `TK_IgnoreImplicitCastsAndParentheses` instead of > `TK_IgnoreUnlessSpelledInSource`? All you seem to be doing is showing that > the former is broken. I've always thought we should remove the former. `AsI

[PATCH] D89468: [libTooling] Change `after` range-selector to operate only on source ranges

2020-10-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: tdl-g. Herald added a project: clang. ymandel requested review of this revision. Currently, `after` fails when applied to locations in macro arguments. This change projects the subrange into a file source range and then applies `after`. R

[PATCH] D89468: [libTooling] Change `after` range-selector to operate only on source ranges

2020-10-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 298392. ymandel added a comment. cleaned up test code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89468/new/ https://reviews.llvm.org/D89468 Files: clang/lib/Tooling/Transformer/RangeSelector.cpp clang/u

[PATCH] D89468: [libTooling] Change `after` range-selector to operate only on source ranges

2020-10-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/unittests/Tooling/RangeSelectorTest.cpp:196 +static void testAfterMacroArg(StringRef Code) { + const std::string Ref = "ref"; tdl-g wrote: > If this helper function took an "expected" parameter I might consider

[PATCH] D89468: [libTooling] Change `after` range-selector to operate only on source ranges

2020-10-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG65cb4fdd69f4: [libTooling] Change `after` range-selector to operate only on source ranges (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

2020-11-20 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: tdl-g. Herald added a project: clang. ymandel requested review of this revision. Currently, `node` only includes the semicolon for (some) statements. However, declarations have the same issue of trailing semicolons, so `node` should behave t

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

2020-11-20 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 306707. ymandel added a comment. Clarified that semicolons are only removed if present. Fixed test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91872/new/ https://reviews.llvm.org/D91872 Files: clang-tools

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

2020-11-20 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D91872#2408278 , @aaron.ballman wrote: > Drive-by question from the peanut gallery, sorry if this is an ignorant one > -- not all declarations have a trailing semicolon; is that handled properly? > e.g., `int x;` has a traili

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

2020-11-20 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG88e62085624e: [libTooling] Update Transformer's `node` combinator to include the trailing… (authored by ymandel). Changed prior to commit: https:/

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-11-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/lib/AST/ASTTypeTraits.cpp:138 +return ASTNodeKind(NKI_##A##Attr); +#include "clang/Basic/AttrList.inc" + } aaron.ballman wrote: > Oye, this brings up an interesting point. Plugin-based attributes currently >

[PATCH] D92340: [libTooling] Remove deprecated Clang Transformer declarations

2020-11-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: tdl-g. Herald added a project: clang. ymandel requested review of this revision. A number of declarations were leftover after the move from `clang::tooling` to `clang::transformer`. This patch removes those declarations and upgrades the hand

[PATCH] D92340: [libTooling][NFC] Remove deprecated Clang Transformer declarations

2020-11-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfdff677a9557: [libTooling] Remove deprecated Clang Transformer declarations (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D92658: [libTooling] Add `formatNode` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: tdl-g. ymandel requested review of this revision. Herald added a project: clang. This new stencil combinator is intended for use in diagnostics and the like. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D92658 Files:

[PATCH] D92658: [libTooling] Add `formatNode` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 309542. ymandel added a comment. renamed the combinator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92658/new/ https://reviews.llvm.org/D92658 Files: clang/include/clang/Tooling/Transformer/Stencil.h cla

[PATCH] D92658: [libTooling] Add `nodeAsString` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 309590. ymandel added a comment. updated name, comments and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92658/new/ https://reviews.llvm.org/D92658 Files: clang/include/clang/Tooling/Transformer/Sten

[PATCH] D92658: [libTooling] Add `describe` stencil for formatting AST nodes for diagnostics.

2020-12-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. thanks! Comment at: clang/unittests/Tooling/StencilTest.cpp:513 +TEST(StencilToStringTest, DescribeOp) { + auto S = describe("Id"); tdl-g wrote: > Can you add a comment (or a more detailed test name) explaining what this > test case

[PATCH] D92658: [libTooling] Add `describe` stencil for formatting AST nodes for diagnostics.

2020-12-07 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe6bc4a71e345: [libTooling] Add `describe` combinator for formatting AST nodes for diagnostics. (authored by ymandel). Changed prior to commit: htt

[PATCH] D89961: [libTooling] Add function to Transformer to create a no-op edit.

2020-10-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added reviewers: tdl-g, gribozavr2. Herald added a project: clang. ymandel requested review of this revision. This functionality is commonly needed in clang tidy checks (based on transformer) that only print warnings, without suggesting any edits. The no-op e

[PATCH] D89961: [libTooling] Add function to Transformer to create a no-op edit.

2020-10-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f8f5cb77efd: [libTooling] Add function to Transformer to create a no-op edit. (authored by ymandel). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

2020-10-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:135 + New->getAllocatedType(), *Result.Context); + if (!Initiali

[PATCH] D82278: Fix traversal over CXXConstructExpr in Syntactic mode

2020-10-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/lib/AST/Expr.cpp:2822 + if (SE->getSourceRange() == E->getSourceRange()) +return Cast->getSubExpr(); +} nit: just `return SE`? Comment at: clang/unittests/ASTMatchers/ASTMatchers

[PATCH] D102175: [clang-tidy] performance-unnecessary-copy-initialization: Remove the complete statement when the copied variable is unused.

2021-05-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Hi Felix, Can you clarify your concern over the warning? Is it the case that the warning is not present before the fix and is only triggered after the fix? I'm concerned that removing the call may have unintended side effects and I would think it might better to leave

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

2021-05-27 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/include/clang/ASTMatchers/GtestMatchers.h:55-59 +/// Matcher for gtest's `ON_CALL` macro. When `Args` is `NoMatchers`, +/// this matches a mock call to a method without argument matchers e.g. +/// `ON_CALL(mock, TwoParamMethod)`; w

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added a comment. Nice! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103195/new/ https://reviews.llvm.org/D103195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D102175: [clang-tidy] performance-unnecessary-copy-initialization: Remove the complete statement when the copied variable is unused.

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:541 Orig.constMethod(); + UnnecessaryCopy.constMethod(); }

[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:97 + // Search the whole function body for decl statements of the initialization + // variable not just the current block statement. auto Matches =

[PATCH] D103087: [clang-tidy] performances-unnecessary-* checks: Extend isOnlyUsedAsConst to expressions and catch const methods returning non-const references/pointers.

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. I have some concerns about the cost of this checks as it used matching over entire contexts quite extensively. At this point, the facilities involved seem quite close to doing dataflow analysis and I wonder if you might be better off with a very different implementatio

[PATCH] D103195: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd0e159334f9d: Add matchers for gtest's ASSERT_THAT, EXPECT_THAT, ON_CALL and EXPECT_CALL (authored by zhaomo, committed by ymandel). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103565/new/ https://reviews.llvm.org/D103565 __

[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Actually, the removal of the default case in the switch gives this warning: ...llvm-project/clang/lib/ASTMatchers/GtestMatchers.cpp:102:11: warning: enumeration value 'Assert' not handled in switch [-Wswitch] I'll reinstate the default case before comitting the patch.

[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb2c8bcbab8a4: Fix "control reaches end of non-void function" warnings on ppc64le (authored by zhaomo, committed by ymandel). Changed prior to commit

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

2021-09-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Looks good! Please add a test to be sure it compiles/works correctly. Thanks! Comment at: clang/include/clang/AST/ASTTypeTraits.h:56 /// Construct an identifier for T. - template - static ASTNodeKind getFromNodeKind() { + template static ASTNode

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

2021-10-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:207 + auto matches = match(traverse(TK_AsIs, typeLoc().bind("tl")), context); + for (auto &nodes : matches) { +

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

2021-10-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG56e72a40c198: Update `DynTypedNode` to support the conversion of `TypeLoc`s. (authored by jcking1034, committed by ymandel). Repository: rG LLVM G

<    3   4   5   6   7   8   9   10   11   12   >