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

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: > Add

[PATCH] D82310: Add `BoolLiteralExpression` to SyntaxTree

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 b

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

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: clang/unittests/Tooling/CMakeLists

[PATCH] D82312: Add `CharLiteral` to SyntaxTree

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/ https

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

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 liter

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

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] D82226: Add Metadata to Transformer tooling

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 che

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

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 https://reviews.llvm.org/

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

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] D82654: [libTooling] Improve error message from failure in selection Stencil

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

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: clang/include/clang/AST/RecursiveASTV

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

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] D82636: Work around a bug in MSVC in the syntax tree test

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/ https://re

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

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: clang/unittes

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

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 im

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

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: clan

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

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 comme

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

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] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

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: clang/unittests/Tooling/CMakeLists.txt

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

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: clang/include/clang/Testing/TestClangConfi

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

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] D82486: RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

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 clang/l

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

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: clang/include/clang/AST/RecursiveASTVisi

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

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 https://reviews.llvm.org/D8

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

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&id=274050#toc Reposi

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

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: clang/inc

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

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 at

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

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 COMPI

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

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 Index

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

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 SINC

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

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] D82760: RecursiveASTVisitor: inline a macro that is only used once

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/ h

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

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 I

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

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, StmtCallbacks_TraverseUnar

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

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: clang/unittests/Tooling/RecursiveAST

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

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 clang/l

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

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 clang/u

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

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 clang/u

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

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 clang/un

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

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] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

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 on

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

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: clang-tools-extra/clang-tidy/modernize/LoopConv

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

gribozavr2 added a comment. Sorry, this change broke a couple bots that are in the official CI, at least http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/7566 and http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/66618. I'm reverting this chang

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

gribozavr2 added a comment. Reverted in https://github.com/llvm/llvm-project/commit/96717125e852d1c6ddf41c22dd2d556f4f5aa34d. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75665/new/ https://reviews.llvm.org/D75665 _

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

gribozavr2 added a comment. Sorry -- this change broke overload resolution for `operator new`, as it is declared in system headers. I'm reverting the patch. $ cat /tmp/in.cu.cc #define __device__ __attribute__((device)) void* operator new(__SIZE_TYPE__ size); __device__ void *operator ne

[PATCH] D84975: [clangd][WIP] Make use of SyntaxTrees for SemanticSelection

gribozavr2 added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:51 +syntax::Leaf *leafContaining(const syntax::Token *Tok, syntax::Tree *Root, + syntax::Arena &A, const SourceManager &SM) { + if (!Tok) It

[PATCH] D85146: [SyntaxTree] Fix crash on pointer to member function

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:4076-4077 + R"cpp( +struct X{ + struct Y{}; +}; Please add spaces before `{`. ===

[PATCH] D85185: [SyntaxTree] Add test coverage for `->*` operator

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:2330 +int X::*pmi; +void test(X x, X y, X* xp) { x = y; Could you add `pmi` as a funct

[PATCH] D85295: [SyntaxTree] Implement the List construct.

gribozavr2 added a comment. I feel uneasy about adding this code without tests. Could we maybe port the function parameter list to use this infrastructure, and then add tests that exercise `getElementsAsNodesAndDelimiters`? Comment at: clang/include/clang/Tooling/Syntax/Tree.

[PATCH] D85305: [SyntaxTree] Remove dead code on dump functions

gribozavr2 added a comment. > Node::dumpTokens was never used. I think it is because it was meant to be a debugger-only helper. I think we should keep it. However, simplifying `static void dumpTokens` to only take one token instead of `ArrayRef` makes sense! Repository: rG LLVM Github Monor

[PATCH] D85316: [SyntaxTree] Proposition of new tree dump

gribozavr2 added a comment. I'd suggest to drop the `<>` quotes (because the AST dump does not add quotes unless it is printing a multi-word thing, and because `<>` don't exactly scream "role" helping to read the output). I'd also suggest to replace multiple spaces before `<>` with a single spa

[PATCH] D85316: [SyntaxTree] Proposition of new tree dump

gribozavr2 added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85316/new/ https://reviews.llvm.org/D85316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D85330: [SyntaxTree] Extend the syntax tree dump

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Tree.cpp:158 if (!Marks.empty()) OS << Marks << ": "; Maybe the marks should be moved after the primary identifier of the node, WDYT? That would be more consistent with AST dump: fi

[PATCH] D85295: [SyntaxTree] Implement the List construct.

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:197 + MaybeTerminated, + Separated, +}; eduucaldas wrote: > gribozavr2 wrote: > > Add a "WithoutDelimiters" case as well? > I think we might want to treat non-delimited-list

[PATCH] D85295: [SyntaxTree] Implement the List construct.

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:247 + + TerminationKind getTerminationKind(); + I just realized that the rest of the syntax tree API does not use the `get~` prefix. However, most of Clang does, as far as I

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

gribozavr2 added a comment. Please also add appropriate handling to `syntax::List::getDelimiterTokenKind`, `getTerminationKind`, and `canBeEmpty`. Comment at: clang/lib/Tooling/Syntax/Nodes.cpp:240 return Children; -} +}; Repository: rG LLVM Github M

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

gribozavr2 added a comment. Could you add unit tests for methods in List and NNS? OK if they are in a separate patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85440/new/ https://reviews.llvm.org/D85440 ___

[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:227-254 +namespace llvm { +template <> struct DenseMapInfo { + using FirstInfo = DenseMapInfo; + using Second

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

gribozavr2 added a comment. LGTM after my comments are addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85440/new/ https://reviews.llvm.org/D85440 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D85295: [SyntaxTree] Implement the List construct.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:194 +/// A Tree that represents a syntactic list of elements. +/// """ A list of elements

[PATCH] D84781: [SyntaxTree] Use PointerUnion instead of inheritance for alternative clauses in NNS

gribozavr2 added a comment. Seeing the API, I like the inheritance-based approach better. It seems more uniform. Comment at: clang/lib/Tooling/Syntax/Nodes.cpp:219 + syntax::EmptyNode *, syntax::Leaf *, syntax::DecltypeSpecifier *, + syntax::SimpleTemplateSpecifier *

[PATCH] D84005: Introduce ns_error_domain attribute.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/test/AST/ast-print-attr.c:20 + +2@interface NSString +@end Stray "2". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84005/new

[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/AST/NestedNameSpecifier.h:20 #include "clang/Basic/SourceLocation.h" +#include "llvm/ADT//DenseMapInfo.h" #include "llvm/ADT/FoldingSet.h" C

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Tree.cpp:362 + switch (this->kind()) { + case NodeKind::NestedNameSpecifier: { +return clang::tok::coloncolon; Please drop the braces within 'case' unl

[PATCH] D85713: [SyntaxTree]: Use Annotations in tests to reduce noise

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:186-187 +auto AnnotatedRanges = AnnotatedCode.ranges(); +assert(AnnotatedRanges.size() == TreeDumps.size()); +for (auto i = 0u; i < AnnotatedRanges.size(); i++) { + auto *An

[PATCH] D85733: [libTooling] Cleanup and reorder `RewriteRule.h`.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:147 +// Every rewrite rules is triggered by a match against some AST node. +// Transformer gua

[PATCH] D85734: [libTooling] Move RewriteRule include edits to ASTEdit granularity.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:39 + // Inserts an include in the file. The `Replacement` field is the name of the + // file for which to add an include. + AddInclude, --

[PATCH] D85750: [SyntaxTree] Unbox operators into tokens for nodes generated from `CXXOperatorCallExpr`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1016 // operand because it does not correspond to anything written in source // code + if (c

[PATCH] D85750: [SyntaxTree] Unbox operators into tokens for nodes generated from `CXXOperatorCallExpr`

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1024 + // representation of built-in and user-defined operators. + if (child->getBeginLoc() == S->getOperatorLoc()) +continue; eduucaldas wrote: > Here we want

[PATCH] D85819: [SyntaxTree] Split tests

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Please also consider splitting the file into multiple. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1238 +namespace n { +template +struct ST {

[PATCH] D85898: [SyntaxTree] Clean `#includes` in `TreeTestBase.h`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.h:14 #include "clang/AST/ASTConsumer.h" -#include "clang/AST/Decl.h" -#include "clang/AST/Stmt.h" #inclu

[PATCH] D85897: [SyntaxTree] Split `TreeTest.cpp`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:16 +namespace clang { +namespace syntax_test { namespace { Ditto, please use namesp

[PATCH] D84005: Introduce ns_error_domain attribute.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa5b8757506b0: Introduce ns_error_domain attribute. (authored by MForster, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84005/new/

[PATCH] D85913: [SyntaxTree] Split `TreeTestBase` into header and source

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:38 +namespace { +static ArrayRef tokens(syntax::Node *N) { + assert(N->isOriginal() && "tokens of mod

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:197 + + bool failed = false; + auto AnnotatedRanges = AnnotatedCode.ranges(); Comment at: clang/unittests/Tooling/Syntax/TreeTestBase.cpp:199 + au

[PATCH] D85962: [SyntaxTree] Create annotations infrastructure and apply it in expression tests.

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Very nice improvement to tests! Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:784 + +TEST_P(SyntaxTreeTest, QualifiedId_ComplexDeclaration) { + if (!G

[PATCH] D86139: [SyntaxTree] Split tests related to Namespace

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2151 + +TEST_P(SyntaxTreeTest, Namepace_UsingDirective) { + if (!GetParam().isCXX()) { -

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

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM, but I don't understand why this option is called "SourceNamingStyle". Existing ClangTidy checkers call it "IncludeStyle". Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

gribozavr2 added a comment. > 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, it is _this_ file's header rather than some arbitrary > header. ... and the results

[PATCH] D79440: Fix ForRangeCopyCheck not triggering on iterators returning elements by value in C++17.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3adaa97f0157: Fix ForRangeCopyCheck not triggering on iterators returning elements by value… (authored by gribozavr). Changed prior to commit: https://reviews.llvm.org/D79440?vs=262202&id=262307#toc Re

[PATCH] D79494: [clang-tidy] Exclude function calls in std namespace for bugprone-argument-comment.

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:67 + // leading underscores in parameter names (libstdc++), suggesting + // them does more harm than good. + unless(hasDeclarati

[PATCH] D79942: [clang] Add an API to retrieve implicit constructor arguments.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4c09289f635c: [clang] Add an API to retrieve implicit constructor arguments. (authored by mboehme, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D132795: [clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp:64 void AssignmentInIfConditionCheck::report(const Expr *MatchedDecl) { +

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:302 +AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}}); +assert(InsertSuccess); }; Please add

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

gribozavr2 added inline comments. Comment at: clang/test/Sema/warn-documentation.cpp:78 +/// Aaa bbb +int test_html_img4(int); + Could you also add these tests to clang/test/Index/comment-to-html-xml-conversion.cpp to show that the whole tag is captured correct

[PATCH] D132713: [clang-tidy] Skip union-like classes in use-equals-default

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:220 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) { - // Skip unions since constructors with empty bodies behave differently - // in comparison wi

[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp:2 // RUN: %check_clang_tidy %s cppcoreguidelines-pr

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

gribozavr2 added inline comments. Comment at: clang/tools/libclang/CXString.cpp:85 + if (String.empty()) +return createEmpty(); + Please split this change into a separate patch and add a unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/tools/libclang/CXString.cpp:85 + if (String.empty()) +return createEmpty(); + egorzhdan wrote: > gribozavr2 wrote: > > Ple

[PATCH] D133009: [libclang] Fix conversion from `StringRef` to `CXString`

gribozavr2 added inline comments. Comment at: clang/test/Index/comment-lots-of-unknown-commands.c:3 +// XFAIL: * + egorzhdan wrote: > This test was never properly passing. Because of the bug in string > conversion, the printed comments contained the entire sou

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

gribozavr2 added a comment. In D133029#3761344 , @ilya-biryukov wrote: > [...] I don't think there is an easy way to write it outside `Sema` and > `Sema` is not readily available for `clang-tidy`. What information is missing, exactly? Repository: r

[PATCH] D128359: [clang][dataflow] Move logic for `createStorageLocation` from `DataflowEnvironment` to `DataflowAnalysisContext`.

This revision was automatically updated to reflect the committed changes. Closed by commit rG12c7352fa488: [clang][dataflow] Move logic for `createStorageLocation` from… (authored by wyt, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D128363: [clang][dataflow] Implement functionality for flow condition variable substitution.

This revision was automatically updated to reflect the committed changes. Closed by commit rGbdfe556dd837: [clang][dataflow] Implement functionality for flow condition variable… (authored by wyt, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:240-241 + // Index used to avoid recreating pointer values for null pointers of the + // same canonical pointee type. + // -

[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.

This revision was automatically updated to reflect the committed changes. Closed by commit rGb611376e7eb5: [clang][dataflow] Singleton pointer values for null pointers. (authored by wyt, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D128658: [clang][dataflow] Do not allow substitution of true/false boolean literals in `buildAndSubstituteFlowCondition`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa34210fa69f: [clang][dataflow] Do not allow substitution of true/false boolean literals in… (authored by wyt, committed by gribozavr). Repository:

[PATCH] D128659: [clang][dataflow] Add `buildAndSubstituteFlowCondition` to `DataflowEnvironment`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae90bc846758: [clang][dataflow] Add `buildAndSubstituteFlowCondition` to `DataflowEnvironment` (authored by wyt, committed by gribozavr). Repositor

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:78 +public: + UncheckedOptionalAccessDiagnosis( + ASTContext &AstContext, UncheckedOptionalAccessModelOptions O

[PATCH] D128352: [clang][dataflow] Use diagnosis API in optional checker

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:67 +std::move(StmtDiagnostics.begin(), StmtDiagnostics.end(), + std::back_inserter(Diagnostics

[PATCH] D129097: [clang][dataflow] Handle null pointers of type std::nullptr_t

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:61 + auto CanonicalPointeeType = + PointeeType.isNull() ? PointeeType : PointeeType

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