[PATCH] D82787: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

2020-07-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7b0be962d681: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post… (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-07-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG94454442c3c1: RecursiveASTVisitor: dont call WalkUp unnecessarily in post-order traversal (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

2020-07-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 275064. gribozavr added a comment. Added release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82921/new/ https://reviews.llvm.org/D82921 Files:

[PATCH] D82875: Added tests for RecursiveASTVisitor for AST nodes that are special cased

2020-07-01 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:501 WalkUpFromStmt IntegerLiteral(1) -WalkUpFromBinaryOperator BinaryOperator(+) - WalkUpFromExpr BinaryOperator(+) -

[PATCH] D82787: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

2020-07-01 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:649 + case BO_##NAME: \ +if (isSameMethod(::TraverseBin##NAME,

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: rsmith, sammccall, ymandel, aaron.ballman. gribozavr updated this revision to Diff 274643. gribozavr added a comment. Removed obsolete comments. This feature was

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274643. gribozavr added a comment. Removed obsolete comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82921/new/ https://reviews.llvm.org/D82921 Files:

[PATCH] D82889: Make RecursiveASTVisitor call WalkUpFrom for operators when the data recursion queue is absent

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:416 +if (!Queue && getDerived().shouldTraversePostOrder()) \ + TRY_TO(WalkUpFromUnary##NAME(S)); \ return true;

[PATCH] D82889: Make RecursiveASTVisitor call WalkUpFrom for operators when the data recursion queue is absent

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: eduucaldas, ymandel, rsmith. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82889 Files: clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D82787: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274495. gribozavr added a comment. Rebased the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82787/new/ https://reviews.llvm.org/D82787 Files: clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D82787: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274497. gribozavr added a comment. Rebased the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82787/new/ https://reviews.llvm.org/D82787 Files: clang/include/clang/AST/RecursiveASTVisitor.h

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

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274481. gribozavr added a comment. Rebased the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82486/new/ https://reviews.llvm.org/D82486 Files: clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D82875: Added tests for RecursiveASTVisitor for AST nodes that are special cased

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: ymandel, eduucaldas. gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at:

[PATCH] D82875: Added tests for RecursiveASTVisitor for AST nodes that are special cased

2020-06-30 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:398 -TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinaryOperator) { +TEST(RecursiveASTVisitor,

[PATCH] D82787: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82787 Files: clang/include/clang/AST/RecursiveASTVisitor.h clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

[PATCH] D82760: RecursiveASTVisitor: inline a macro that is only used once

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG58f2be9671a8: RecursiveASTVisitor: inline a macro that is only used once (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82760/new/

[PATCH] D82766: Compile the RecursiveASTVisitor callbacks test with "/bigobj"

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1cf2e45c19ff: Compile the RecursiveASTVisitor callbacks test with /bigobj (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82766/new/

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. I reverted your commit because it seemed to have broken the build: FalsePositiveRefutationBRVisitorTest.cpp:112:3: error: use of undeclared identifier 'LLVM_WITH_Z3' https://github.com/llvm/llvm-project/commit/a44425f25b5ca417e7ecee6e7e00040224e50a69 CHANGES

[PATCH] D82766: Compile the RecursiveASTVisitor callbacks test with "/bigobj"

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274108. gribozavr added a comment. Order lines alphabetically. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82766/new/ https://reviews.llvm.org/D82766 Files: clang/unittests/Tooling/CMakeLists.txt

[PATCH] D82766: Compile the RecursiveASTVisitor callbacks test with "/bigobj"

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/CMakeLists.txt:10 if (MSVC) + set_source_files_properties(RecursiveASTVisitorTests/Callbacks.cpp PROPERTIES COMPILE_FLAGS /bigobj) set_source_files_properties(RecursiveASTVisitorTest.cpp PROPERTIES

[PATCH] D82766: Compile the RecursiveASTVisitor callbacks test with "/bigobj"

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. gribozavr2 added a reviewer: erichkeane. erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment

[PATCH] D82760: RecursiveASTVisitor: inline a macro that is only used once

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: eduucaldas, ymandel. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82760 Files: clang/include/clang/AST/RecursiveASTVisitor.h Index:

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG339ed1e042c0: Move TestClangConfig into libClangTesting and use it in AST Matchers tests (authored by gribozavr). Changed prior to commit: https://reviews.llvm.org/D82179?vs=274008=274050#toc

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8e5a56865f28: Add tests for sequences of callbacks that RecursiveASTVisitor produces (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274032. gribozavr added a comment. Added a FIXME about a regression. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82486/new/ https://reviews.llvm.org/D82486 Files:

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274027. gribozavr added a comment. Rebased the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82486/new/ https://reviews.llvm.org/D82486 Files: clang/include/clang/AST/RecursiveASTVisitor.h

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274010. gribozavr added a comment. Simplified capture lists in lambdas Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 274008. gribozavr added a comment. Addressed code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82179/new/ https://reviews.llvm.org/D82179 Files:

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked 5 inline comments as done. gribozavr2 added a comment. > However, is this worth an RFC to the list? I don't think so. I'm consolidating existing testing infrastructure that was already providing this functionality, and using regular parameterized tests (`TEST_P`).

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

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:117 + recordDefaultImplementation( + __func__, [&, this]() { RecordingVisitorBase::WalkUpFromStmt(S); }); +

[PATCH] D82157: Fix crash on `user defined literals`

2020-06-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:632 +// As a node is built by folding tokens we cannot generate one +// for `_w` +Builder.markChildToken(S->getBeginLoc(), syntax::NodeRole::LiteralToken); This

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 273835. gribozavr added a comment. Unified recordCallback and recordDefaultImplementation into one call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. > Now, in all the test cases we are calling the default implementation. We are > not surfacing that WalkUpFrom can not walk up. I think we are. The log of callbacks called from the default implementation is indented to the right, so we clearly see what the default

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 273834. gribozavr added a comment. Removed an unused argument from recordDefaultImplementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

[PATCH] D82636: Work around a bug in MSVC in the syntax tree test

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfa1b48877618: Work around a bug in MSVC in the syntax tree test (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82636/new/

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:646 + * receives a DataRecursionQueue, can't call WalkUpFrom after traversing \ + * children because it only enqueues the children.

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 273736. gribozavr added a comment. Removed a stray period in a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82486/new/ https://reviews.llvm.org/D82486 Files:

[PATCH] D82654: [libTooling] Improve error message from failure in selection Stencil

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. > I generally avoid testing error message content in tests, but I know there's > a variety of opinions on this subject... If you thought that the quality of the error message matters enough to improve it, then it is worth testing the message, I think. Repository:

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked 2 inline comments as done. gribozavr2 added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:335 + template struct is_same_method_impl { +static bool isSameMethod(...) { return false; } + }; ymandel wrote: > Why

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

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 273723. gribozavr added a comment. Addressed review comments, added more fixes that must be committed in the same change, because splitting them would break tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82226: Add Metadata to Transformer tooling

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. In D82226#2115406 , @asoffer wrote: > I think the tradeoff here is > Dynamic typing -- faster compile times, type safety checked at run-time (in > tests), lower maintenance cost > Templates -- Faster runtime, type safety

[PATCH] D82654: [libTooling] Improve error message from failure in selection Stencil

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Any chance for a test? Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:244 + llvm::make_error_code(errc::invalid_argument) && +

[PATCH] D82312: Add `CharLiteral` to SyntaxTree

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. @vvereschaka Sorry about that. It looks like a bug in MSVC. I implemented a workaround in https://reviews.llvm.org/D82636 -- please review if you have time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82312/new/

[PATCH] D82636: Work around a bug in MSVC in the syntax tree test

2020-06-26 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: eduucaldas, vvereschaka. MSVC does not handle raw string literals with embedded double quotes correctly. I switched the affected test case to use regular string

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

2020-06-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 273464. gribozavr added a comment. Added calls to default implementations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files:

[PATCH] D82310: Add `BoolLiteralExpression` to SyntaxTree

2020-06-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1240 + true; + false; +} eduucaldas wrote: > gribozavr2 wrote: > > eduucaldas wrote: > > > gribozavr2 wrote: > > > > C99 has

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

2020-06-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 273302. gribozavr added a comment. Addressed code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82485/new/ https://reviews.llvm.org/D82485 Files: clang/unittests/Tooling/CMakeLists.txt

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

2020-06-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked 5 inline comments as done. gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp:16 +template +class RecordingVisitorBase : public TestVisitor { + bool VisitPostOrder; ymandel wrote: >

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

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. gribozavr2 added reviewers: ymandel, eduucaldas. These tests show a bug: post-order traversal introduces an extra call to WalkUp*, that is not present in pre-order traversal. I'm fixing

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

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: eduucaldas, ymandel. How does RecursiveASTVisitor call the WalkUp callback for expressions? - In pre-order traversal mode, RecursiveASTVisitor calls the WalkUp

[PATCH] D82312: Add `CharLiteral` to SyntaxTree

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM after new tests are added. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1240 + 'a'; + L'a'; +} Could you add tests for escape

[PATCH] D82360: Add StringLiteral to SyntaxTree

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM with improved testing. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1237 + "a\n"; + L"a"; +} Could you add some non-ASCII

[PATCH] D82310: Add `BoolLiteralExpression` to SyntaxTree

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1240 + true; + false; +} eduucaldas

[PATCH] D82318: Add `FloatingLiteral` to SyntaxTree

2020-06-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1239-1242 +1e-2; +2.; +.2; +2.f; Indent -2. Repository: rG LLVM

[PATCH] D82310: Add `BoolLiteralExpression` to SyntaxTree

2020-06-22 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1240 + true; + false; +} C99 has bool literals, but the program should include stdbool.h. I feel like it is better to make the predicate something like "hasBoolType()"

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

2020-06-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added subscribers: cfe-commits, sstefan1, jfb. Herald added a reviewer: jdoerfert. Herald added a project: clang. gribozavr2 added a reviewer: ymandel. gribozavr2 edited the summary of this revision. Previously, AST Matchers tests were using a custom way to

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

2020-06-18 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1b2f6b4a08ba: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions (authored by eduucaldas, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

2020-06-18 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/Nodes.cpp:182 + for (auto *C = firstChild(); C; C = C->nextSibling()) { +if (C->role() ==

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

2020-06-18 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Transformer/Parsing.cpp:169 + +// For consistency with matcher parser and C++ code, node id's are written as +// strings. However,

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

2020-06-18 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:187 +/// A sequence of these specifiers make a `nested-name-specifier` +class NameSpecifier final : public Tree { Could you provide examples or a grammar quote in the

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

2020-06-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Transformer/Parsing.h:1 +//===--- Parsing.h - Parsing library for Transformer -*- C++ -*-===// +// Please pad the first line to match the other line (throughout the patch).

[PATCH] D81444: Make the diagnostic-missing-prototypes put the suggested `static` in front of `const` if exists.

2020-06-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1302 +bool Lexer::isConst(SourceLocation Loc, const SourceManager , +const LangOptions ) { I'm concerned about putting this API into Lexer, given this implementation. It

[PATCH] D81396: [clang-tidy] New util `Aliasing` factored out from `bugprone-infinite-loop`

2020-06-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/utils/Aliasing.h:29 +/// pointer to ``n`` created in ``f()``. + +bool hasPtrOrReferenceInFunc(const FunctionDecl *Func,

[PATCH] D81444: Make the diagnostic-missing-prototypes put the suggested `static` in front of `const` if exists.

2020-06-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14252 + FD->getReturnType().isConstQualified()) +return FD->getReturnTypeSourceRange().getBegin().getLocWithOffset( +/*strlen("const ")=*/-6);

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

2020-06-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:209 +/// qualified-id: +/// nested-name-specifier template_opt unqualified-id +class IdExpression final : public Expression { Please add a TODO for the accessor for the

[PATCH] D81396: [clang-tidy] New util `Aliasing` factored out from `bugprone-infinite-loop`

2020-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Aliasing.cpp:59 + +/// Return whether `Var` has a pointer or reference in `Func`. +bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var) { Please don't

[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

2020-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:190 +return N->kind() == NodeKind::NameSpecifier; + } +}; Should there be getters for various parts of the specifier? Comment at:

[PATCH] D81180: AST Matchers test: use arrays instead of vectors

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa180d5409f21: AST Matchers test: use arrays instead of vectors (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81180/new/

[PATCH] D81180: AST Matchers test: use arrays instead of vectors

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. > probably didn't need a review this one I don't believe in post-commit review anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81180/new/ https://reviews.llvm.org/D81180

[PATCH] D81157: Propose naming principle for NodeRole and apply it

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG42f6fec3878d: Propose naming principle for NodeRole and apply it (authored by eduucaldas, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D81157?vs=268519=268543#toc

[PATCH] D81150: Use libClangTesting in the unittest for AST matchers

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:62 +inline ArrayRef langCxx11OrLater() { + static std::vector Result = {Lang_CXX11, Lang_CXX14, Lang_CXX17, +

[PATCH] D81180: AST Matchers test: use arrays instead of vectors

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added a reviewer: njames93. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D81180 Files: clang/unittests/ASTMatchers/ASTMatchersTest.h Index:

[PATCH] D81157: Propose naming principle for NodeRole and apply it

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:110 +/// BinaryOperatorExpression_leftHandSide can only appear as a child of a +/// BinaryOperatorExpression node.end with `NodeKind_` of the parent enum class NodeRole : uint8_t {

[PATCH] D81168: Add support for id-expression in SyntaxTree

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:815 +| | `-UnknownExpression +| | `-s +| `-; This part does not seem to appear in the source. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D81157: Propose naming principle for NodeRole and apply it

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:124 IfStatement_thenStatement, IfStatement_elseKeyword, IfStatement_elseStatement, eduucaldas wrote: > gribozavr2 wrote: > > Shouldn't `elseKeyword` have no prefix?

[PATCH] D81150: Use libClangTesting in the unittest for AST matchers

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb5fc1deb5ba1: Use libClangTesting in the unittest for AST matchers (authored by gribozavr). Changed prior to commit: https://reviews.llvm.org/D81150?vs=268440=268495#toc Repository: rG LLVM Github

[PATCH] D81155: Rename arrow -> arrowToken for unified naming

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG62305f6db4ed: Rename arrow - arrowToken for unified naming (authored by eduucaldas, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D81157: Propose naming principle for NodeRole and apply it

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:94 /// a binary expression'. Used for implementing accessors. +// How to name NodeRole: +// If the child node is a token/keyword, end its name with 'Token'/'Keyword' I'd

[PATCH] D81150: Use libClangTesting in the unittest for AST matchers

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 268440. gribozavr added a comment. Changed C++20-only tests to C++20-or-later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81150/new/ https://reviews.llvm.org/D81150 Files:

[PATCH] D81150: Use libClangTesting in the unittest for AST matchers

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added subscribers: cfe-commits, sstefan1, mgorny. Herald added a reviewer: jdoerfert. Herald added a project: clang. The unittest for AST matchers has its own way to specify language standards. I unified it with the shared infrastructure from

[PATCH] D81135: Add support for IntegerLiteral in SyntaxTree

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3b739690b01e: Add support for IntegerLiteral in SyntaxTree (authored by eduucaldas, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D81135?vs=268428=268435#toc Repository:

[PATCH] D81135: Add support for IntegerLiteral in SyntaxTree

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:642 // TODO: add accessors for specifiers. - syntax::Leaf *arrow(); + syntax::Leaf *arrowToken(); syntax::SimpleDeclarator *declarator(); Could you move this change

[PATCH] D81135: Add support for IntegerLiteral in SyntaxTree

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:718 +} +)cpp", + R"txt( Could you remove the whitespace in front of `)cpp"` for

[PATCH] D81107: Make syntax tree test print the line number when it fails

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG06cf7adcc881: Make syntax tree test print the line number when it fails (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81107/new/

[PATCH] D81135: Add support for IntegerLiteral in SyntaxTree

2020-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:190 + } + syntax::Leaf *literal(); +}; "literalToken"? Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:706 +void test() { + 42; +}

[PATCH] D81107: Make syntax tree test print the line number when it fails

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: hlopko. Herald added a project: clang. Herald added a subscriber: cfe-commits. The syntax tree test uses a helper function that executes all testing assertions. When an assertion fails, the only line number that gets printed to the log

[PATCH] D81092: Add support for `nullptr` in SyntaxTrees

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG007098d7e6b8: Add support for `nullptr` in SyntaxTrees (authored by eduucaldas, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D81087: Replaced C++2a with C++20 in clang-tools-extra

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc1911fcb0664: Replaced C++2a with C++20 in clang-tools-extra (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81087/new/

[PATCH] D81087: Replaced C++2a with C++20 in clang-tools-extra

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. gribozavr added a reviewer: hlopko. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, kbarton, nemanjai. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D81087 Files:

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. This change broke the build: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/29716. The build is already fixed by https://github.com/llvm/llvm-project/commit/fd2740143e626ca32432aac0b51b2880a3b1e0bc. Please always run `ninja check-all` before

[PATCH] D81040: Split syntax tree tests into more granular ones

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd7d5dd31fc6f: Split syntax tree tests into more granular ones (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81040/new/

[PATCH] D81040: Split syntax tree tests into more granular ones

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 268104. gribozavr added a comment. Address code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81040/new/ https://reviews.llvm.org/D81040 Files: clang/unittests/Tooling/Syntax/TreeTest.cpp

[PATCH] D81019: Syntax tree: ignore implicit expressions at the top level of statements

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb34b7691facd: Syntax tree: ignore implicit expressions at the top level of statements (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D81040: Split syntax tree tests into more granular ones

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 268100. gribozavr added a comment. Refreshing this patch after pushing the patch that this one depends on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81040/new/ https://reviews.llvm.org/D81040 Files:

[PATCH] D81019: Syntax tree: ignore implicit expressions at the top level of statements

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1048 + syntax::Tree *ChildNode; + if (Expr *ChildExpr = dyn_cast(Child)) { +// This is an expression in a statement position, consume the trailing eduucaldas wrote: > I

[PATCH] D81040: Split syntax tree tests into more granular ones

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2066 +int func3b(int *); +int func4(int a, float b); +int func4a(int, float); hlopko wrote: > func4 -> func4a > func4a -> func4b? Fixed, thanks. Repository: rG LLVM

[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present

2020-06-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Nice find, thank you for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___

[PATCH] D81040: Split syntax tree tests into more granular ones

2020-06-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: hlopko, eduucaldas. Doing so allows us to increase test coverage by removing unnecessary language restrictions. Repository: rG LLVM Github Monorepo

[PATCH] D81019: Syntax tree: ignore implicit expressions at the top level of statements

2020-06-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr updated this revision to Diff 267937. gribozavr added a comment. Refactored makeStmtChild to have fewer redundant checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81019/new/ https://reviews.llvm.org/D81019 Files:

[PATCH] D81019: Syntax tree: ignore implicit expressions at the top level of statements

2020-06-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: hlopko, eduucaldas. I changed `markStmtChild` to ignore implicit expressions the same way as `markExprChild` does it already. The test that I modified crashes

<    1   2   3   4   5   6   7   8   9   10   >