[PATCH] D87374: [SyntaxTree] Specialize `TreeTestBase` for `BuildTreeTest` and `MutationsTest`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/Syntax/MutationsTest.cpp:48 + +TEST_P(MutationTest, Mutations) { + if (!GetParam().isCXX11OrLater()) { ==

[PATCH] D87249: [SyntaxTree] Fix crash on functions with default arguments.

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:4177 + auto x1 = [[X(1)]]; + auto x2 = [[X(1, '2')]]; +} Please also add tests tha

[PATCH] D87249: [SyntaxTree] Fix crash on functions with default arguments.

gribozavr2 added a comment. Please also add tests for calls to constructors without arguments and calls to implicit constructors 1 argument, as requested in https://reviews.llvm.org/D86700. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87249/new/

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1137 +// Ignore the implicit default constructs. +if ((S->getNumArgs() == 0 || isa(S->getArg(0))) && +S->getParenOrBraceRange().isInvalid()) eduucaldas wrote: > g

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:48 +// Ignores the implicit `CXXConstructExpr` for copy/move constructors generated +// by the compiler, as well as in implicit conversions like the one wrapp

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1136 + bool WalkUpFromCXXConstructExpr(CXXConstructExpr *S) { +// Ignore the implicit default constructs. +if ((S->getNumArgs() == 0 || isa(S->getArg(0

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

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:388 +llvm::Expected> +rewriteDescendants(const Decl &Node, RewriteRule Rule, + const ast_matchers::MatchFinder::MatchResult &Result); ymandel wro

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

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:388 +llvm::Expected> +rewriteDescendants(const Decl &Node, RewriteRule Rule, +

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

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:48-58 +static Expr *IgnoreImplicitCXXConstructExpr(Expr *E) { + if (auto *C = dyn_cast(E)) { +auto NumArgs

[PATCH] D86700: [SyntaxTree] Ignore leaf implicit `CXXConstructExpr`

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:552 [[::n::S s1]]; [[n::S s2]]; } Do we have tests for calling constructors w

[PATCH] D86778: Extract infrastructure to ignore intermediate expressions into `clang/AST/IgnoreExpr.h`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/AST/IgnoreExpr.h:15 +namespace clang { +namespace { +/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *, ---

[PATCH] D86778: Extract infrastructure to ignore intermediate expressions into `clang/AST/IgnoreExpr.h`

gribozavr2 added a comment. > This allows users to use IgnoreExprNodes outside of clang/AST/Expr.h Did you mean "outside of Expr.cpp"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86778/new/ https://reviews.llvm.org/D86778 __

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:202 +/// location of the `^`: +/// `int ^a;` +/// `int ^a::S::f(){}` Comment at: clang/lib/Tooling/Syntax/BuildTree

[PATCH] D85214: [OpenMP] Ensure testing for versions 4.5 and default - Part 3

gribozavr2 added a comment. In D85214#2243167 , @saiislam wrote: > In D85214#2243160 , @gribozavr2 > wrote: > >> @saiislam This change seems to have broken a buildbot, please take a look at >> your earliest conven

[PATCH] D85214: [OpenMP] Ensure testing for versions 4.5 and default - Part 3

gribozavr2 added a comment. @saiislam This change seems to have broken a buildbot, please take a look at your earliest convenience. Before: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35354 After: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/35355 Re

[PATCH] D86719: [SyntaxTree][NFC] Refactor function templates into functions taking base class

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. > Perhaps we should inline getQualifiedNameStart and getInitializerRange and > make getDeclaratorRange a member function taking a Decl. What do you think? These helpers seem to have we

[PATCH] D86679: [SyntaxTree][NFC] Append "get" to syntax Nodes accessor names

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. > It's worth noting that accessors in the base APIs don't follow this rule. > Should we refactor them as well? I'd say yes. > In this patch? Up to you. Repository: rG LLVM Github

[PATCH] D86636: [SyntaxTree] Refactor `NodeRole`s

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:156 // Roles specific to particular node kinds. - OperatorExpression_operatorToken, - UnaryOperatorExp

[PATCH] D86600: [SyntaxTree] Migrate `ParamatersAndQualifiers` to use the new List API

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:4041 +template +[[void test(T , Args... );]] +)cpp", Could you also add one with a n

[PATCH] D86600: [SyntaxTree] Migrate `ParamatersAndQualifiers` to use the new List API

gribozavr2 added a comment. Could you add some tests with variadic parameter lists & variadic function templates? If we don't handle them correctly, please leave a FIXME in the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86600/new/ https:/

[PATCH] D86600: [SyntaxTree] Migrate `ParamatersAndQualifiers` to use the new List API

gribozavr2 added a comment. I think we also need to update `List::getTerminationKind()` and other similar List methods to handle this list kind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86600/new/ https://reviews.llvm.org/D86600

[PATCH] D86544: [SyntaxTree] Add support for call expressions

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:333 +/// call-arguments: +/// delimited_list(expression, ',', ) +/// Note: This construct is a simpli

[PATCH] D86441: [SyntaxTree] Split ExplicitTemplateInstantiation test

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:3054 -TEST_P(SyntaxTreeTest, ExplicitTemplateInstantations) { +TEST_P(SyntaxTreeTest, ExplicitTempl

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

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:107-110 int main() { if (1) {} if (1) {} else if (0) {} } eduucaldas wrote: > I just noticed that we didn't yet use annotations on statement tests. > I think t

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

gribozavr2 added a comment. Output format LGTM! Comment at: clang/lib/Tooling/Syntax/Tree.cpp:160 + assert(N); + if (auto *L = llvm::dyn_cast(N)) { +OS << "'"; Comment at: clang/lib/Tooling/Syntax/Tree.cpp:169 - auto *T = cast(N); -

[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`

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:911 + S->getEndLoc()), + idExpression, nullptr); + ---

[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:180 + MemberExpression_member, + MemberExpression_accessToken, }; Could you order new items in source order? object, access token, member. Comment at: c

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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. 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D84005: Introduce ns_error_domain attribute.

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3650 S.Diag(Field->getLocation(), diag::warn_transparent_union_attribute_field_size_align) << isSize << *Field << FieldBits; --

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:76 + /// Stores the style options for a given directory. + mutable llvm::StringMap>

[PATCH] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. > Passed test cases but failed in the real world as std::string has a non > trivial destructor so creates a CXXBindTemporaryExpr. An idea for a future change: move the std::string mock

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:224 + else +consumeError(ValueOr.takeError()); + return llvm::None; Is this specialization defined only because parsing a string option can never fail? I'd let th

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:124 +static void +populateNaimgStyle(std::vector &Styles, + const ClangTidyCheck::OptionsView &Options) { I think it would be bette

[PATCH] D84315: [libTooling] Add a `between` range-selector combinator.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Transformer/RangeSelector.h:59 +/// Convenience constructor of the range between two ranges. +inline RangeSelector betwe

[PATCH] D84005: Introduce ns_error_domain attribute.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/AttrDocs.td:3340 + +const char *MyErrorDomain; +typedef NS_ERROR_ENUM(unsigned char, MyErrorEnum, MyErrorDomain) { -

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:295 syntax::NestedNameSpecifier *qualifier(); - // TODO after expose `id-expression` from `DependentScopeDeclRefExpr`: // Add accessor for `template_opt`. + syntax::Leaf *templateKeyw

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

gribozavr2 added a comment. What is this diff based on? On the left I see, for example, NamespaceNameSpecifier, which is not in the repository yet. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:777 // FIXME: Support Microsoft's __super - return new (allocator(

[PATCH] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

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:337 +// refer to nodes bound by the calling rule. `Rule` is not applied to the node +// itself.

[PATCH] D83966: Enable the test for hasArraySize() AST matcher in all language modes

This revision was automatically updated to reflect the committed changes. Closed by commit rGb6073ee9ae84: Enable the test for hasArraySize() AST matcher in all language modes (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D839

[PATCH] D84005: Introduce ns_error_domain attribute.

gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1860 +def NSErrorDomain : Attr { + let Spellings = [GNU<"ns_error_domain">]; + let Args = [IdentifierArgument<"ErrorDomain">]; aaron.ballman wrote: > MForster wrote: > > gribozavr2

[PATCH] D84005: Introduce ns_error_domain attribute.

gribozavr2 added a comment. Did your latest update unintentionally drop the test file `clang/test/Analysis/ns_error_enum.m`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84005/new/ https://reviews.llvm.org/D84005 __

[PATCH] D84005: Introduce ns_error_domain attribute.

gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1860 +def NSErrorDomain : Attr { + let Spellings = [GNU<"ns_error_domain">]; + let Args = [IdentifierArgument<"ErrorDomain">]; Could we try to add a list of subjects here? It seems

[PATCH] D83966: Enable the test for hasArraySize() in all language modes

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In C++11 and later Clang generates an implicit conversion from int to size_t in the AST. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D83966 Files: clang/unittests/ASTMat

[PATCH] D83868: Use TestClangConfig in AST Matchers tests and run them in more configurations

This revision was automatically updated to reflect the committed changes. Closed by commit rG4f244c4b42b0: Use TestClangConfig in AST Matchers tests and run them in more configurations (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D83868: Use TestClangConfig in AST Matchers tests and run them in more configurations

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I am changing tests for AST Matchers to run in multiple language standards versions, and under multiple triples that have different behavior with regards to templates. This change is similar to

[PATCH] D83820: Change metadata to deferred evalutaion in Clang Transformer.

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:268 -inline ASTEdit withMetadata(ASTEdit edit, llvm::Any Metadata) { - edit.Metadata = std::m

[PATCH] D83700: Fix test for the hasExternalFormalLinkage matcher

This revision was automatically updated to reflect the committed changes. Closed by commit rG8978032a17cd: Fix test for the hasExternalFormalLinkage matcher (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83700/new/ https://re

[PATCH] D83700: Fix test for the hasExternalFormalLinkage matcher

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Names of local variables have no linkage (see C++20 [basic.link] p8). Names of variables in unnamed namespace have internal linkage (see C++20 [basic.link] p4). Repository: rG LLVM Github M

[PATCH] D83513: [AST][ObjC] Fix crash when printing invalid objc categories

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/AST/DeclPrinterTest.cpp:180 + return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.m", +

[PATCH] D83529: Summary: [clang] Provide a way for WhileStmt to report the location of its LParen and RParen.

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Could you take a look at test failures and check if they are relevant? `linux > Clang.AST::ast-dump-attr.cpp` looks extremely close to the area you're working on. I'm not quite comfor

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

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:749 + else +return new (allocator()) syntax::FloatUserDefinedLiteralExpression; +} ---

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

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:737 + Builder.findToken(TokLoc)->text(Context.getSourceManager()); + auto Literal = NumericLiteralParser{TokSpelling, + TokLoc,

[PATCH] D83480: Refactored NumericLiteralParser to not require a Preprocessor

This revision was automatically updated to reflect the committed changes. Closed by commit rG3cca818efabb: Refactored NumericLiteralParser to not require a Preprocessor (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83480/new/

[PATCH] D83480: Refactored NumericLiteralParser to not require a Preprocessor

gribozavr2 added inline comments. Comment at: clang/include/clang/Lex/LiteralSupport.h:57-59 - NumericLiteralParser(StringRef TokSpelling, - SourceLocation TokLoc, - Preprocessor &PP); eduucaldas wrote: > We don't need

[PATCH] D83480: Refactored NumericLiteralParser to not require a Preprocessor

gribozavr updated this revision to Diff 276740. gribozavr added a comment. Addressed code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83480/new/ https://reviews.llvm.org/D83480 Files: clang/include/clang/Lex/LiteralSupport.h

[PATCH] D83480: Refactored NumericLiteralParser to not require a Preprocessor

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We would like to use NumericLiteralParser in the implementation of the syntax tree builder, and plumbing a preprocessor there seems inconvenient and superfluous. Repository: rG LLVM Github M

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

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:634 +// `SourceLocation`s. As a result one of these nodes has a valid +// `SourceLocation` that doesn't point to a token. +// eduucaldas wrote: > gribozavr2 wrote: >

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:820 + // A postfix unary operator is declared as taking two operands. The + // second operand is used to differ from its prefix counterpart. In the +

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

gribozavr2 added a comment. > Fix crash on `user defined literals` WDYT: Implement support for user defined literals (which also fixes a crash) > Given an UserDefinedLiteral 1.2_w: > Problem: Lexer generates one Token for the literal, but ClangAST > references two source locations > Fix: Ign

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2192 R"cpp( +class osstream {}; struct X { eduucaldas wrote: > gribozavr2 wrote: > > eduucaldas wrote: > > > gribozavr2 wrote: > > > > I don't think we need a separa

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2192 R"cpp( +class osstream {}; struct X { eduucaldas wrote: > gribozavr2 wrote: > > I don't think we need a separate class to show the left shift operator. The > >

[PATCH] D83293: [clang-tidy] Fix an unused-raii check crash on objective-c++.

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/bugprone-unused-raii-crash.mm:3 + +struct Foo { + ~Foo() {} Foo => CxxClass ABC => ObjcC

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

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:149 + case OO_PipePipe: + // Less common binary operators + case OO_ArrowStar: Please drop

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG5689b38c6a42: Removed a RecursiveASTVisitor feature to visit operator kinds with different… (authored by gribozavr). Changed prior to commit: https://reviews.llvm.org/D82921?vs=275064&id=275601#toc Rep

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:118 +syntax::NodeKind getOperatorNodeKind(const CXXOperatorCallExpr &E) { + switch (E.getOperator()) { eduucaldas wrote: > # Where to put this logic? > The pro of having this

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG5689b38c6a42: Removed a RecursiveASTVisitor feature to visit operator kinds with different… (authored by gribozavr). Changed prior to commit: https://reviews.llvm.org/D82921?vs=275064&id=275663#toc Rep

[PATCH] D82937: Fix `isInfixBinaryOp` that returned true for postfix ++

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/CXXOperatorCallExprTest.cpp:21 + const std::string Code = R"cpp( + struct X{ +friend X operator+(X, X); Please add a space before "{". Comment at: clang/unittests/Tooli

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG79889691430d: Added tests for RecursiveASTVisitor for AST nodes that are special cased (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG8bf4c40af813: Make RecursiveASTVisitor call WalkUpFrom for operators when the data recursion… (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

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