[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 tho

[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: cl

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

[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] 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/ https://revi

[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 pushi

[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: clang-tools-extra/

[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/ https://revie

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

2020-07-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D83966: Enable the test for hasArraySize() in all language modes

2020-07-16 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84005: Introduce ns_error_domain attribute.

2020-07-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84005: Introduce ns_error_domain attribute.

2020-07-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
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.

2020-07-17 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D83966: Enable the test for hasArraySize() AST matcher in all language modes

2020-07-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

2020-07-23 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/include/clang/Tooling/Transformer/RewriteRule.h:337 +// refer to nodes bound by the calling rule. `Rule` is not applied to the node +// itself.

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

2020-07-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84348: WIP: Add complete id-expression support to syntax trees

2020-07-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84005: Introduce ns_error_domain attribute.

2020-07-27 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/include/clang/Basic/AttrDocs.td:3340 + +const char *MyErrorDomain; +typedef NS_ERROR_ENUM(unsigned char, MyErrorEnum, MyErrorDomain) { -

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

2020-07-27 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/include/clang/Tooling/Transformer/RangeSelector.h:59 +/// Convenience constructor of the range between two ranges. +inline RangeSelector betwe

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

2020-07-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

2020-07-29 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. > 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] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-29 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/readability/IdentifierNamingCheck.h:76 + /// Stores the style options for a given directory. + mutable llvm::StringMap>

[PATCH] D84005: Introduce ns_error_domain attribute.

2020-07-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

2020-04-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:239 + + auto It = llvm::partition_point(F.Mappings, [SpelledI](const Mapping &M) { +return M.BeginSpelled <= SpelledI; It would be great to add an is_sorted assertion to the bu

[PATCH] D77461: [clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

2020-04-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. I expect different style guides to have a different opinion on this, depending on the justification for the rule. > The purpose of the rule is to avoid code which causes hidden dependencies. That's one justification, which would prohibit such static variables. Anoth

[PATCH] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

2020-04-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:264 + auto *FrontMapping = mappingStartingBeforeSpelled(File, &Spelled.front()); + unsigned SpelledFrontI = &Spelled.front() - File.SpelledTokens.data(); + unsigned ExpandedBegin; ---

[PATCH] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

2020-04-07 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1bf055c9891f: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer (authored by hlopko, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D77209?vs=255654&id=255

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

2020-04-07 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/include/clang/Tooling/Transformer/RewriteRule.h:40 + +/// A map from a match result to a list of concrete errors (with possible +/// failure). T

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

2020-04-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 marked an inline comment as done. gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:44 +/// of `EditList`. +using EditList = MatchConsumer>; + ymandel wrote: > griboza

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

[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 r

[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-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/ ht

[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] 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 t

[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&id=268435#toc Repository:

[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 libClangTesting

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

[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 sug

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

[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&id=268495#toc Repository: rG LLVM Github

[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] 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 A

[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] 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: clang/unittests/AST

[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&id=268543#toc Repos

[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] 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/ https://rev

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

[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 duplicate

[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 'te

[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); Could

[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, c

[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 &SM, +const LangOptions &LangOpts) { I'm concerned about putting this API into Lexer, given this implemen

[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] 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 comme

[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(&RecursiveASTVisitor::TraverseBin#

[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(+) -W

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

[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 https://reviews.l

[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: don't call WalkUp unnecessarily in post-order traversal (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

2020-07-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2020-07-03 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

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

2020-07-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

2020-07-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

2020-07-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2020-07-07 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

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

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

2020-07-07 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/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

2020-07-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

2020-07-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D82157: Fix crash on `user defined literals`

2020-07-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2020-07-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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`

2020-07-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D83480: Refactored NumericLiteralParser to not require a Preprocessor

2020-07-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D83480: Refactored NumericLiteralParser to not require a Preprocessor

2020-07-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2020-07-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2020-07-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D82157: Fix crash on `user defined literals`

2020-07-09 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D82157: Fix crash on `user defined literals`

2020-07-10 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/BuildTree.cpp:749 + else +return new (allocator()) syntax::FloatUserDefinedLiteralExpression; +} ---

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

2020-07-10 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. 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] D83513: [AST][ObjC] Fix crash when printing invalid objc categories

2020-07-10 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/AST/DeclPrinterTest.cpp:180 + return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.m", +

[PATCH] D83700: Fix test for the hasExternalFormalLinkage matcher

2020-07-13 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D83700: Fix test for the hasExternalFormalLinkage matcher

2020-07-14 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] D83820: Change metadata to deferred evalutaion in Clang Transformer.

2020-07-15 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/include/clang/Tooling/Transformer/RewriteRule.h:268 -inline ASTEdit withMetadata(ASTEdit edit, llvm::Any Metadata) { - edit.Metadata = std::m

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

2020-07-15 Thread Dmitri Gribenko via Phabricator via cfe-commits
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] 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, w

[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() == syntax::NodeRole::NestedNameSp

[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 ht

[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] 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()" a

[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 Githu

[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 wrote:

[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 character

[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 seque

[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 call

[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 t

[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 c

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