[PATCH] D62524: [clang-tidy] Fix description for misc-definitions-in-headers.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp:133 +<< IsFullSpec << FD; +diag(FD->getLocation(), /*FixDescription=*/"make

[PATCH] D62616: [CodeComplete] Add a bit more whitespace to completed patterns

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. I totally agree about the space before the opening curly, but the space before the opening paren is a matter of style. Certainly it does match LLVM style better, but it does not match s

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

gribozavr added a comment. In D67140#1664106 , @NoQ wrote: > In D67140#1659982 , @gribozavr wrote: > > > We should take a page from desktop software here. If the messages were in a > > separate file, there would be

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

gribozavr added a comment. Reverted in r371598. Another concern that I have is cross-compilation. LLVM's ADT is not set up to be cross-compiled like the rest of compiler-rt is. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66

[PATCH] D67419: [analyzer] NFC: Move PathDiagnostic to libAnalysis.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:26 #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Config/config.h" #include "clang/Format/F

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

gribozavr added a comment. Are you planning to move users of these options -- like `PlistDiagnostics` -- to libAnalysis? Comment at: clang/include/clang/Analysis/PathDiagnostic.h:81 + /// because deterministic mode is always superior when done right, but + /// for some consu

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

gribozavr requested changes to this revision. gribozavr added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Analysis/PathDiagnosticConsumers.h:37 const cross_tu::CrossTranslationUnitContext &CTU); -#include "clang

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

gribozavr added a comment. The "bugtype" in IssueHash is specific to Static Analyzer (or at least not documented sufficiently abstractly). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67421/new/ https://reviews.llvm.org/D67421 ___

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

gribozavr added a comment. `PathDiagnosticConsumerOptions` is pretty lightweight, and is not passed around in hot paths if I understand correctly. What do you think about passing it by value everywhere? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67420/new/ https://reviews.llvm.org/

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

gribozavr added inline comments. Comment at: clang/include/clang/Analysis/PathDiagnosticConsumers.def:23 +ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer) +ANALYSIS_DIAGNOSTICS(SARIF, "sar

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

gribozavr added inline comments. Comment at: clang/include/clang/Analysis/IssueHash.h:18 /// Get an MD5 hash to help identify bugs. /// Returns an opaque identifier for a diagnostic. This opaque identifier is intended to be stable even when the source code i

[PATCH] D67452: [clang] [unittest] Import LLVMTestingSupport if necessary

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks for the fix! I'm not a fan of the complexity that standalone builds create, but if we are supporting them... oh well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67452/

[PATCH] D67491: Removed some questionable default arguments from setters

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. They can be confusing -- what does it mean to call a setter without a value? Also, some setters, like `setPrintTemplateTree` had `false` as the default value! The callers are largely not using

[PATCH] D67491: Removed some questionable default arguments from setters

This revision was automatically updated to reflect the committed changes. Closed by commit rL371731: Removed some questionable default arguments from setters (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: h

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. FWIW, I like this approach better than the one in Static Analyzer because it warns on the variable declaration -- that's where the root of the issue is, not at the call to `dispatch_once

[PATCH] D67744: [clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit constructors.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67744/new/ https://reviews.llvm.org/D67744 __

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/file-filter.cpp:60 +// CHECK4-QUIET: header.h:1:12: warning: single-argument constructors must be marked explicit +// CHECK5: header.h:3:12: warning: single-argument constructors must be marked expli

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks! Please let me know if you need me to commit the patch for you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67501/new/ https://reviews.llvm.org/D67501 __

[PATCH] D67632: [libTooling] Introduce new library of source-code builders.

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/SourceCodeBuilders.h:9 +/// +/// /file +/// This file collects facilities for generating source-code strings. "\file" Comment at: clang/include/clang/Tooling/R

[PATCH] D67621: [libTooling] Add `ifBound`, `elseBranch` RangeSelector combinators.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Refactoring/RangeSelector.cpp:320 + }; +} May I ask to keep the implementation order consistent with the header file

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

This revision was automatically updated to reflect the committed changes. Closed by commit rL372388: [clang-tidy] Fix relative path in header-filter. (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

[PATCH] D67632: [libTooling] Introduce new library of source-code builders.

gribozavr added inline comments. Comment at: clang/lib/Tooling/Refactoring/SourceCodeBuilders.cpp:68 +std::string tooling::buildParens(const Expr &E, const ASTContext &Context) { + StringRef ExprText = getText(E, Context); + if (mayNeedParens(E)) ymandel wrote:

[PATCH] D67633: [libTooling] Add `access` and `ifBound` combinators to Stencil library.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:70 +// describing a member m, yields "e->m", when e is a pointer, "e2->m" when e = +// "*e2" and "e.m" otherwi

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

gribozavr accepted this revision. gribozavr added a comment. Thanks! Please let me know if you need me to commit the patch for you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64736/new/ https://reviews.llvm.org/D64736 ___ cfe-commits mai

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

gribozavr added a comment. Sorry, I reverted this patch in r372601. Unfortunately, it makes paths printed in clang-tidy'd diagnostics inconsistent with what `-header-filter` operates on. For example, imagine that `file-filter.cpp` includes `header_alias.h`, which is a symlink to `header.h`. Th

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

gribozavr added a subscriber: ilya-biryukov. gribozavr added a comment. I added a test in r372607 for your reference. It tests both `foo/..` and symlink behavior. > Maybe only collapsing `foo/..` would be viable? @ilya-biryukov told me that even that is not viable, in case `foo` is a symlink t

[PATCH] D67961: [libTooling] Introduce the MatchConsumer abstraction

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Refactoring/MatchConsumer.h:9 +/// +/// /file This file defines the *MatchConsumer* abstraction: a computation over +/// mat

[PATCH] D67969: [libTooling] Add `run` combinator to Stencils.

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:181 +/// This supports user-defined extensions to the Stencil language. +StencilPart run(MatchConsumer C); + We could reimplement all other stencils through `run()` and

[PATCH] D67969: [libTooling] Add `run` combinator to Stencils.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:181 +/// This supports user-defined extensions to the Stencil language. +StencilPart run(MatchConsumer

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

gribozavr added a comment. Thanks! Do you need me to commit the patch for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67567/new/ https://reviews.llvm.org/D67567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads

gribozavr added a comment. What is the expected contract of the functions that this checker flags? Are they supposed to perform unaligned reads correctly, and we have just an implementation bug in these functions, or is it the caller's fault if they pass an unaligned address? ===

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

gribozavr added a comment. Sorry, could you rebase the patch to apply cleanly to master? Seems like someone else edited ReleaseNotes.rst in the meanwhile. $ arc patch D67567 ... Checking patch clang-tools-extra/docs/ReleaseNotes.rst... error: while searching for: Finds instances wher

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

gribozavr added a comment. Sorry, I reverted it in r373032 because the test fails on Linux: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18323 . Could you please take a look? Thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67567/new/ h

[PATCH] D68251: [clang-tidy] Fix module registry name and description for Darwin clang-tidy module.

This revision was automatically updated to reflect the committed changes. Closed by commit rL373304: [clang-tidy] Fix module registry name and description for Darwin clang-tidy… (authored by gribozavr, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D68315: [libTooling] Add various Stencil combinators for expressions.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:149 /// \returns the source corresponding to the selected range. StencilPart selection(RangeSelector

[PATCH] D68315: [libTooling] Add various Stencil combinators for expressions.

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:149 /// \returns the source corresponding to the selected range. StencilPart selection(RangeSelector Selector); ymandel wrote: > gribozavr wrote: > > Should the com

[PATCH] D68315: [libTooling] Add various Stencil combinators for expressions.

gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:149 /// \returns the source corresponding to the selected range. StencilPart selection(RangeSelector Selector); ymandel

[PATCH] D68574: [libTooling] Add `toString` method to the Stencil class

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:54 + /// Constructs a string representation of the StencilPart. StencilParts + /// generated by the

[PATCH] D68637: [libTooling] Move Transformer files to their own directory/library.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Update header guards? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68637/new/ https://reviews.llvm.org/D68637

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

gribozavr added a comment. It looks to me that a better fix is to fix the checker to not emit this warning in MS compatibility mode. I'm OK with "fixing" the test like this, however, it should come with a TODO. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.l

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

gribozavr added a comment. I'm holding off on reviewing the code until we figure out what the rules are. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:10 +The check conservatively tries to preserve logical costness in favor of

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

gribozavr added a comment. > There's no shortage of possible implicit TODOs I don't see "-fno-ms-compatibility" as an implicit TODO. It is most commonly used as "this test does something that does not work in MS mode". When I read it, I can't tell why it is there. When other people write tests,

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

gribozavr added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.llvm.org/D68640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

gribozavr added a reviewer: gribozavr. gribozavr requested changes to this revision. gribozavr added a comment. This revision now requires changes to proceed. +1 to what MyDeveloperDay said. The infrastructure can't know whether the newlines are intentional or not. Checks that edit the source cod

[PATCH] D61675: [WIP] Update IRBuilder::CreateFNeg(...) to return a UnaryOperator

gribozavr added a comment. Sorry, but this commit broke OCaml tests: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/19014 I reverted it in r374354. Please test before re-landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D616

[PATCH] D68795: [libTooling] Introduce a single, "main" header file for Transformer.

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:28 +// also update their code to reference `TransformerTool`. +#include "clang/Tooling/Transformer/TransformerTool.h" +// Temporary alias to assist clients in migrating to new class

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

gribozavr created this revision. Herald added subscribers: cfe-commits, lebedev.ri, jfb, arphaman, mgrang, christof, kbarton, aheejin, nemanjai, srhines. Herald added a reviewer: jfb. Herald added a reviewer: jdoerfert. Herald added a reviewer: lebedev.ri. Herald added a project: clang. gribozavr

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

gribozavr added a comment. Why does the number of moves matter? Git preserves history across moves. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68807/new/ https://reviews.llvm.org/D68807 ___ cfe-comm

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

gribozavr added a comment. It would look weird if we have a ton of tests for checkers in a directory, and then a subdirectory for infra tests. Why are we trying to optimize the number of renamed files? I already did the renaming, so we are not saving any work. I think we should optimize for lo

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

gribozavr added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:10 +The check conservatively tries to preserve logical costness in favor of +physical costness. The only operations on ``this`` that this check consid

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

This revision was automatically updated to reflect the committed changes. Closed by commit rG885c559369fe: [ClangTidy] Separate tests for infrastructure and checkers (authored by gribozavr). Changed prior to commit: https://reviews.llvm.org/D68807?vs=224378&id=224569#toc Repository: rG LLVM

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

gribozavr added a comment. > I guess here's the high-level question: should all removals that remove all > non-blank text from a line also delete the line? I see your point, but as https://llvm.org/PR43583 shows, we have a much larger problem: textual replacements don't compose. So, whatever we

[PATCH] D68876: [libTooling] Group all Transformer combinators in a single namespace.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. WDYT about `clang::transformer`? I don't see much point in the intermediate namespace. However, LGTM either way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D68876: [libTooling] Group all Transformer combinators in a single namespace.

gribozavr added a comment. > What do you think of my just putting all of the Transformer types + combis in > the single clang::transformer namespace? That would make sense to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68876/new/ https://re

[PATCH] D56924: Special case ObjCPropertyDecl for printing

gribozavr added inline comments. Comment at: lib/AST/Decl.cpp:1543 +Ctx = ID; + } Like you said in a private conversation, yes, support for `ObjCIvarDecl` also seems necessary. Comment at: unittests/AST/NamedDeclPrinterTest.cpp:220

[PATCH] D68885: Add a Ranges field to Diagnostic struct

gribozavr added a comment. Please upload patches with context in future (`arc diff` will do that for you). Please also merge this patch with the user, https://reviews.llvm.org/D68887. No need to add untested code. Comment at: include/clang/Tooling/Core/Diagnostic.h:88 + + +

[PATCH] D69184: [libTooling] Introduce general combinator for fixed-value `MatchConsumer`s.

gribozavr added a comment. What are the use cases for non-text values? I like the name `text` much better... If the name conflict is the issue, I think you could rename it to `toText` -- it would even read more fluently `change(toText("abc"))`, also making it obvious that we are not changing *t

[PATCH] D69171: [clang-fuzzer] Add new fuzzer target for Objective-C

gribozavr added a comment. Looks reasonable to me. The duplication is unfortunate, but it is reasonable while we have two binaries. However, you could explore reading the command line flags for HandleCXX in LLVMFuzzerInitialize from fuzzer's command line flags. Repository: rG LLVM Github Mo

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

gribozavr2 added a comment. + Argyrios for libclang changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80126/new/ https://reviews.llvm.org/D80126 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D80239: [libTooling] In Transformer, allow atomic changes to span multiple files.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Transformer/Transformer.cpp:65 - for (const auto &I : Case.AddedIncludes) { -auto &Header = I.first; -switch (I.second) {

[PATCH] D80540: Add support for binary operators in Syntax Trees

This revision was automatically updated to reflect the committed changes. Closed by commit rG3785eb83af41: Add support for binary operators in Syntax Trees (authored by eduucaldas, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D80540?vs=266121&id=266155#toc Reposit

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

gribozavr2 added a comment. Please run clang/docs/tools/dump_ast_matchers.py to update the docs. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7022 + .matches(Node, Finder, Builder); +} + Could you move it closer to other parameter-related matc

[PATCH] D80624: Add support for UnaryOperator in SyntaxTree

This revision was automatically updated to reflect the committed changes. Closed by commit rG461af57de781: Add support for UnaryOperator in SyntaxTree (authored by eduucaldas, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D806

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

gribozavr2 added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2672 +TEST(IsAtPosition, BlockDecl) { + // FIXME: Needs to enable block-support (ie., -fblocks) + if (true) Please use `matchesObjC` / `notMatchesObjC`. Rep

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

gribozavr2 added a comment. In D80603#2058019 , @aaron.ballman wrote: > I'm sorry if I'm being dense, but `hasParameter` traverses to the > `ParmVarDecl`, so I'm not certain I understand why this new matcher is needed > as a public matcher. It seems li

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

gribozavr2 added a reviewer: ymandel. gribozavr2 added a subscriber: ymandel. gribozavr2 added a comment. @ymandel owns transformers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80697/new/ https://reviews.llvm.org/D80697 __

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:234 +DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher InnerMatcher, +

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:154 +const RewriteRule &Rule, +ast_type_traits::TraversalKind DefaultTraversalKind) { // Map the cases into buckets of matchers -- one for eac

[PATCH] D80704: Remove WrapperMatcherInterface

gribozavr created this revision. Herald added subscribers: cfe-commits, arichardson. Herald added a project: clang. gribozavr2 added a reviewer: ymandel. WrapperMatcherInterface is an abstraction over a member variable -- in other words, not much of an abstraction at all. I think it makes code har

[PATCH] D80704: Remove WrapperMatcherInterface

This revision was automatically updated to reflect the committed changes. Closed by commit rG35492270ed70: Remove WrapperMatcherInterface (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80704/new/ https://reviews.llvm.org/D807

[PATCH] D80731: Improve test infrastructure in SyntaxTree

This revision was automatically updated to reflect the committed changes. Closed by commit rGeca41919d28b: Improve test infrastructure in SyntaxTree (authored by eduucaldas, committed by gribozavr). Changed prior to commit: https://reviews.llvm.org/D80731?vs=266898&id=266993#toc Repository:

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

gribozavr2 added a comment. > Is there a reason we want to support blocks but not lambdas? Lambdas are supported (see tests). Lambdas generate classes with regular function decls, so there's no surprise there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

gribozavr2 added a comment. > Also, you need to update Registry.cpp to add this to the list of dynamic > matchers. Good point. @oontvoo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80603/new/ https://reviews.llvm.org/D80603 __

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

gribozavr created this revision. Herald added subscribers: cfe-commits, martong. Herald added a reviewer: shafik. Herald added a reviewer: rengolin. Herald added a project: clang. Declaring these helpers in the ast_matcher namespace in the clangAST library seems inappropriate -- neither these help

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

gribozavr updated this revision to Diff 267187. gribozavr added a comment. Addressed review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80786/new/ https://reviews.llvm.org/D80786 Files: clang/unittests/AST/ASTImporterFixtures.cpp c

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

gribozavr2 added a comment. In D80786#2062557 , @sammccall wrote: > - Unittest -> Test (shorter, and unit test is two words, and llvm naming > conventions notwithstanding many gtests are not unit tests) Done. > - UnitTestClangArgs --> `std::vector` it'

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

gribozavr updated this revision to Diff 267188. gribozavr added a comment. Changed description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80786/new/ https://reviews.llvm.org/D80786 Files: clang/unittests/AST/ASTImporterFixtures.cpp clang/u

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

gribozavr2 added a comment. In D80786#2062634 , @martong wrote: > Just out of curiosity. In what way do you prepare to share these test? For > which component are you planning to reuse this test infrastructure? Tests in clang/unittests/Tooling/Syntax/Tr

[PATCH] D80792: Move unittest helpers to a shared location

gribozavr updated this revision to Diff 267204. gribozavr added a comment. Removed some refactoring leftovers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80792/new/ https://reviews.llvm.org/D80792 Files: clang/include/clang/Testing/CommandLin

[PATCH] D80792: Move unittest helpers to a shared location

gribozavr created this revision. Herald added subscribers: cfe-commits, martong, mgorny. Herald added a project: clang. gribozavr updated this revision to Diff 267204. gribozavr added a comment. Removed some refactoring leftovers. unittests/AST/Language.h defines some helpers that we would like

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

This revision was automatically updated to reflect the committed changes. Closed by commit rGd4ef654673a9: Rename APIs in unittests/AST/Language.h in preparation to share them (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D807

[PATCH] D80792: Move unittest helpers to a shared location

gribozavr updated this revision to Diff 267214. gribozavr added a comment. Added a modulemap entry for the new Clang library. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80792/new/ https://reviews.llvm.org/D80792 Files: clang/include/clang/Tes

[PATCH] D80792: Move unittest helpers to a shared location

gribozavr updated this revision to Diff 267237. gribozavr added a comment. Fixed a clang-tidy warning about header guard format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80792/new/ https://reviews.llvm.org/D80792 Files: clang/include/clang/

[PATCH] D80792: Move unittest helpers to a shared location

This revision was automatically updated to reflect the committed changes. Closed by commit rG0e265e315784: Move unittest helpers to a shared location (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80792/new/ https://reviews.l

[PATCH] D80822: Run syntax tree tests in many language modes

gribozavr created this revision. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. gribozavr2 added reviewers: hlopko, eduucaldas. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D80822 Files: clang/unittests/Tooling/Syntax/CMakeLists.txt clang/unit

[PATCH] D80812: Add support for Overloaded Binary Operators in SyntaxTree

This revision was automatically updated to reflect the committed changes. Closed by commit rG3a574a6cb359: Add support for Overloaded Binary Operators in SyntaxTree (authored by eduucaldas, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D80822: Run syntax tree tests in many language modes

gribozavr updated this revision to Diff 267351. gribozavr added a comment. Rebased the patch on top of the current master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80822/new/ https://reviews.llvm.org/D80822 Files: clang/unittests/Tooling/Sy

[PATCH] D80884: [ASTMatchers] Force c++ unittests to specify correct language standard

gribozavr2 added a comment. Approving since I think that it is probably too costly for tooling to try to handle such language extensions. We also don't have a principled way to know what "future" C++ feature is supported in a given language mode, so I think we would always have implementation a

[PATCH] D80884: [ASTMatchers] Force c++ unittests to specify correct language standard

gribozavr2 added a comment. This is an interesting question. Since Clang accepts certain constructs in older C++ standard versions as extensions, shouldn't we test AST matchers in those modes as well? I think it could be argued either way. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

gribozavr2 added a comment. I think it makes sense to align the documentation and the definition of IgnoreUnlessSpelledInSource, I'm just not sure which component we should change -- the documentation or the implementation. Making IgnoreUnlessSpelledInSource not match template instantiations is

[PATCH] D80822: Run syntax tree tests in many language modes

gribozavr updated this revision to Diff 267814. gribozavr added a comment. Updated according to the code review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80822/new/ https://reviews.llvm.org/D80822 Files: clang/unittests/Tooling/Syntax/CMake

[PATCH] D80822: Run syntax tree tests in many language modes

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:54-63 + bool isCXX() const { +return Language == Lang_CXX || Language == Lang_CXX11 || + Language == Lang_CXX14 || Language == Lang_CXX17 || + Language == Lang_CXX2a

[PATCH] D80822: Run syntax tree tests in many language modes

This revision was automatically updated to reflect the committed changes. Closed by commit rG44f989e78096: Run syntax tree tests in many language modes (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80822/new/ https://reviews

[PATCH] D81009: Reinstate the syntax tree test for 'static' in an array subscript

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added a reviewer: eduucaldas. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D81009 Files: clang/unittests/Tooling/Syntax/TreeTest.cpp Index: clang/unittests/Too

[PATCH] D81009: Reinstate the syntax tree test for 'static' in an array subscript

This revision was automatically updated to reflect the committed changes. Closed by commit rG53c29a42d044: Reinstate the syntax tree test for 'static' in an array subscript (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81009/

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

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

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

gribozavr updated this revision to Diff 267937. gribozavr added a comment. Refactored makeStmtChild to have fewer redundant checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81019/new/ https://reviews.llvm.org/D81019 Files: clang/lib/Tooling

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

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr2 added reviewers: hlopko, eduucaldas. Doing so allows us to increase test coverage by removing unnecessary language restrictions. Repository: rG LLVM Github Monorepo https://revie

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

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Nice find, thank you for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ c

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

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2066 +int func3b(int *); +int func4(int a, float b); +int func4a(int, float); hlopko wrote: > func4 -> func4a > func4a -> func4b? Fixed, thanks. Repository: rG LLVM Gith

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