[PATCH] D66947: Changed FrontendActionFactory::create to return a std::unique_ptr

This revision was automatically updated to reflect the committed changes. Closed by commit rL370379: Changed FrontendActionFactory::create to return a std::unique_ptr (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

gribozavr created this revision. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66960 Files: clang-tools-extra/clangd/unittests/IndexActionTests.cpp clang-tools-extra/unittests

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

gribozavr added a comment. In D66960#1651198 , @lebedev.ri wrote: > This is missing documentation changes. Could you point out such places? I tried to remove "takes ownership" comments which became redundant. > And this likely would be good to mention

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

gribozavr updated this revision to Diff 218046. gribozavr added a comment. Updated documentation and added release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66960/new/ https://reviews.llvm.org/D66960 Files: clang-tools-extra/clangd/un

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

gribozavr added a comment. In D66960#1651258 , @lebedev.ri wrote: > In D66960#1651243 , @gribozavr wrote: > > > In D66960#1651198 , @lebedev.ri > > wrote: > > > > > This is

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

This revision was automatically updated to reflect the committed changes. Closed by commit rL370451: [Tooling] Migrated APIs that take ownership of objects to unique_ptr (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

gribozavr added inline comments. Comment at: clang/lib/AST/Comment.cpp:151 +static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL, + bool testTypedefTypeLoc = false) { TypeLoc PrevTL; Mordante wrote: > gribozavr wrote

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. LGTM, but could you also verify that references are not an issue? using D = void(); D &d = ...; ///< \return none CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66706/new/ h

[PATCH] D66945: [clang-tidy] Fix a false positive in unused-using-decl check.

gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/misc-unused-using-decls.cpp:214 +class Bar {}; +Bar *bar; It is very unclear what this test does, if I didn't know about this bug. Could you add a comment? Repository: rG LLVM Git

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks for the simplification! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:460 + using iterator = ReportList::iterator; + using con

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:460 + using iterator = ReportList::iterator; + using const_iterator = ReportList::const_iterator; + gribozavr wrote: > I don't think we intend users

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:460 + using iterator = ReportList::iterator; + using const_iterator = ReportList::const_iterator; + gribozavr wrote: > gribozavr wrote: > > I don't t

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:350 + + llvm::ArrayRef getFixits() const { return Fixits; } + No need to qualify with "llvm::". Comment at: clang/include/clang/St

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

gribozavr added a comment. Thank you for the conversation so far! This is not a complete review from me, but I'm trying to avoid branching in too many directions at once. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:122 + /// Get the location

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/test/Analysis/dead-stores.c:11 long idx=abc+3*5; // expected-warning {{never read}} expected-warning{{unused variable 'idx'}} + // expected-remark-re@-1.*}}:11 - {{.*}}:18 - ''}} } NoQ wrote: > gribozav

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

gribozavr added a comment. > So, I plan to rework this into two revisions: one to match > https://reviews.llvm.org/D66676 (and keep the tests esssentially as they are) > and one to add getRuleMatchLoc for future use. That SGTM. I don't have an opinion about whether they should be separate revi

[PATCH] D66806: [LifetimeAnalysis] Fix some false positives

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Looks like a reasonable way to suppress some false positives. It will suppress some true positives (e.g., imagine an "identity" function that returns the same pointer as was provided to

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:174 + /// This location is used by clients rendering diagnostics. + virtual PathDiagnosticLocation getLocation(const SourceManager &SM) const { +assert(Location.

[PATCH] D67077: [libclang] Refactored SharedParsedRegionsStorage

gribozavr created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. gribozavr added a reviewer: jkorous. Herald added a subscriber: dexonsmith. Removed the `PPRegionSetTy` typedef because it is only used 3 times, and obscures code more than it helps. R

[PATCH] D66694: [libclang][index][NFCi] Refactor machinery for skipping already parsed function bodies

gribozavr added inline comments. Comment at: clang/tools/libclang/Indexing.cpp:134 void copyTo(PPRegionSetTy &Set) { std::lock_guard MG(Mux); Set = ParsedRegions; jkorous wrote: > gribozavr wrote: > > I think we should lock both the source and destin

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

This revision was automatically updated to reflect the committed changes. Closed by commit rL370677: [Wdocumentation] fixes an assertion failure with typedefed function and block… (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Change

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

gribozavr added a comment. Thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66706/new/ https://reviews.llvm.org/D66706 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D66302: [SVE][Inline-Asm] Support for SVE asm operands

gribozavr added inline comments. Comment at: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp:5836-5837 if (VT.getSizeInBits() == 128) return std::make_pair(0U, &AArch64::FPR128_loRegClass); +case 'y': + if (!Subtarget->hasFPARMv8())

[PATCH] D66945: [clang-tidy] Fix a false positive in unused-using-decl check.

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp:105 } // Mark using declarations as used by setting FoundDecls' value to zero. As // the AST is walked in order, usages are only marked after a the P

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

gribozavr accepted this revision. gribozavr added a comment. Thanks! Yay consistency. I prefer the term "checker" to refer to individual modules because I feel it is more precise and less ambiguous. In phrases like "malloc check", "make_unique check", it is unclear what does the check -- malloc

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

gribozavr added a comment. In D67140#1658315 , @aaron.ballman wrote: > In D67140#1656831 , @NoQ wrote: > > > Honestly, i'm much more worried about message capitalization :) > > > Likewise. I wish the static analyze

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

gribozavr added a comment. In D67140#1658365 , @aaron.ballman wrote: > Ah, good to know! That reduces my concern, but doesn't negate it. AFAIK, we > haven't changed the interface such that it requires code changes rather than > just a recompile in recen

[PATCH] D67077: [libclang] Refactored SharedParsedRegionsStorage

This revision was automatically updated to reflect the committed changes. Closed by commit rL371041: [libclang] Refactored SharedParsedRegionsStorage (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

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

gribozavr added a comment. In D67140#1659356 , @aaron.ballman wrote: > In D67140#1658969 , @gribozavr wrote: > > > In D67140#1658365 , @aaron.ballman > > wrote: > > > > > A

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

gribozavr added a comment. In D67140#1658365 , @aaron.ballman wrote: > In D67140#1658353 , @gribozavr wrote: > > > In D67140#1658315 , @aaron.ballman > > wrote: > > > > > I

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

gribozavr added a comment. We should take a page from desktop software here. If the messages were in a separate file, there would be a lot of people capable of mass-editing them. When messages are hardcoded in the tool code, navigating and editing them requires more skill, and definitely a lot

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

gribozavr accepted this revision. gribozavr added a comment. Still LGTM, just some nitpicks to replace iterator usage with index-based accesses (which I believe is simpler). Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:569

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491 + ArrayRef Ranges = None, + ArrayRef Fixits = None); NoQ wrote: > gribozavr wrote: > > I'm not sure i

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:343 + /// to produce a good fix-it hint for most path-sensitive warnings. + void addFixItHint(const FixItHint &F) { +Fixits.push

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

gribozavr added a comment. Thanks for following up! Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:53 + +/// \brief Return whether `Var` has a pointer of reference in `S`. +static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) { Please d

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks for the quick fix! Comment at: clang-tools-extra/test/clang-tidy/bugprone-use-after-move.cpp:1198 +} + } for (int i = 0; i < 10; ++i) {

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-use-after-move.cpp:1198 +} + } for (int i = 0; i < 10; ++i) { ymandel wrote: > gribozavr wrote: > > Unless you think it is redundant, could you also add > > > > ``` >

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

gribozavr accepted this revision. gribozavr added a comment. I think this patch is a good improvement, and I don't want to hold it back -- but like we discussed before, and like you wrote on the mailing list, I would want a more simple API for ClangTidy. Comment at: clang/in

[PATCH] D71842: Allow newlines in AST Matchers in clang-query files

gribozavr2 added inline comments. Comment at: clang/include/clang/ASTMatchers/Dynamic/Parser.h:167 static llvm::Optional - parseMatcherExpression(StringRef MatcherCode, Sema *S, - const NamedValueMap *NamedValues, - Diagnostics

[PATCH] D72097: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

gribozavr2 added a comment. Could you provide a more fleshed out example of a case where it is useful? Could you update the documentation for the attribute? I'm forgetting if there's a spec for this attribute -- is this extension part of that spec? Do you foresee any issues or further special c

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

gribozavr2 added a comment. Thank you for the patch! Comment at: clang/docs/ReleaseNotes.rst:68 +- -Wdocumentation properly handles Doxygen comments with multiple identifiers in + one \param command. The validation whether the name of the identifier is valid + has been impr

[PATCH] D72446: [Syntax] Build mapping from AST to syntax tree nodes

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:147 + void add(ASTPtr From, syntax::Tree *To) { +assert(To != nullptr); + Also assert tha

[PATCH] D72334: [Syntax] Build nodes for template declarations.

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:930 +Builder.markChildToken(TemplateKW, syntax::NodeRole::IntroducerKeyword); +Builder.markMaybeDelayedChild( +TemplatedDeclaration, Why is this range maybe-delay

[PATCH] D72334: [Syntax] Build nodes for template declarations.

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:190 + // Set role for the node that may or may not be delayed. Node must span + // exactly \p Range. + void

[PATCH] D72073: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

gribozavr2 added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:5556 + SourceLocation StarLoc = Tok.getLocation(); SourceLocation Loc = ConsumeToken(); D.SetRangeEnd(Loc); ConsumeToken() returns Tok.getLocation(), so why do we need

[PATCH] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

gribozavr2 added inline comments. Comment at: clang/test/SemaObjC/attr-swift_objc_members.m:4 +#if !__has_attribute(swift_objc_members) +#error cannot verify precense of swift_objc_members attribute +#endif compnerd wrote: > aaron.ballman wrote: > > gribozavr2 wr

[PATCH] D87495: [SyntaxTree][Synthesis] Add support for simple Leafs and test based on tree dump

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:30-31 +syntax::Leaf *createKeyword(Arena &A, tok::TokenKind K); +syntax::Leaf *createLeaf(syntax::Arena &A, const char *spelling, + tok::TokenKind K); + --

[PATCH] D87495: [SyntaxTree][Synthesis] Add support for simple Leafs and test based on tree dump

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:51 + return createLeaf(A, tok::getKeywordSpelling(K), K); +} + eduucaldas wrote: > gribozavr2 wrote: > > Could we make a combined function that does not require the user to make

[PATCH] D87495: [SyntaxTree][Synthesis] Add support for simple Leafs and test based on tree dump

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:38 +StringRef Spelling) { + auto *Leaf = createLeafLowLevel(A, Spellin

[PATCH] D87533: [SyntaxTree][Synthesis] Add support for Tree.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:39 +syntax::Tree * +createTree(Arena &A, std::vector>, + syntax::NodeKind);

[PATCH] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87395/new/ https://reviews.llvm.org/D87395

[PATCH] D87482: Fix clang Wrange-loop-analysis in BuildTree.cpp

gribozavr2 added a comment. Sorry, I didn't realize you don't have commit access. Pushed this patch to git, thanks for the contribution! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87482/new/ https://reviews.llvm.org/D87482

[PATCH] D87482: Fix clang Wrange-loop-analysis in BuildTree.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe10df779f097: Fix clang Wrange-loop-analysis in BuildTree.cpp (authored by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D87600: [SyntaxTree][List] `assertInvariants` for `List`s

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/Tree.cpp:236 + + auto *L = dyn_cast(T); + if (!L) Comment at: clang/lib/Tooling/Syntax

[PATCH] D87683: [clang-tidy] Crash fix for bugprone-misplaced-pointer-arithmetic-in-alloc

gribozavr2 added a comment. In D87683#2274883 , @martong wrote: > In D87683#2274663 , > @baloghadamsoftware wrote: > >> In D87683#2274569 , @njames93 wrote: >> >>> Please fi

[PATCH] D87749: [SyntaxTree][Synthesis] Implement `deepCopy`

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:45 +/// Deep copies `N`. +/// "Creates a completely independent copy of `N` (a deep

[PATCH] D87895: [SyntaxTree] Test for '\' inside token.

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:173 + R"cpp( +in/ +t a; That looks like the wrong slash to me (forward instead

[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`

gribozavr2 accepted this revision. gribozavr2 added a comment. > This introduces the new swift_name attribute that allows annotating > interfaces with an alternate spelling for Swift. This is used as part > of the importing mechanism to allow interfaces to be imported with a new s/interfaces/APIs

[PATCH] D87779: [SyntaxTree] Test `findFirstLeaf` and `findLastLeaf`

gribozavr2 added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:9 +// +// This file tests the Syntax Tree base API. +// Please elaborate what "base API" covers -- specifically, what classes and such. However, I'm not sure it is necessa

[PATCH] D87720: Sema: add support for `__attribute__((__swift_private__))`

gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3592 + let Content = [{ +Objective-C declarations marked with the ``swift_private`` attribute are hidden +from the framework client but are still made available for use within the ---

[PATCH] D87749: [SyntaxTree][Synthesis] Implement `deepCopy`

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:216 + Child = Child->getNextSibling()) +if (!Child->canModify()) + return false; Shouldn't we call canModifyAllDescendants recursively? Comment

[PATCH] D87749: [SyntaxTree][Synthesis] Implement `deepCopy`

gribozavr2 added inline comments. Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:205 + if (L->canModify()) +syntax::FactoryImpl::setCanModify(Leaf); + eduucaldas wrote: > gribozavr2 wrote: > > Since we are creating new leaves, why prohibit their mutation

[PATCH] D87839: [SyntaxTree] Test the List API

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:224 + /// "a, b c" <=> [("a", ","), ("b", nul), ("c", nul)] + /// "a, b,"<=> [("a", ","), ("b", ","), (nul, nul)] /// I'd slightly prefer "null" b/c "nul" refers to

[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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3386-3388 +The ``swift_objc_members`` attribute maps to the ``@objcMembers`` Swift +attribute, which indicates that Swift members of this class, its subclasses, and +all extensions thereof, will

[PATCH] D87331: Sema: add support for `__attribute__((__swift_error__))`

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3377-3382 +def SwiftDocs : DocumentationCategory<"Controlling Swift Import"> { + let Content = [{ +Clang supports additional attributes for controlling how APIs

[PATCH] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

gribozavr2 added inline comments. Comment at: clang/test/SemaObjC/attr-swift_objc_members.m:4 +#if !__has_attribute(swift_objc_members) +#error cannot verify precense of swift_objc_members attribute +#endif aaron.ballman wrote: > gribozavr2 wrote: > > > The typo

[PATCH] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3387 +The ``swift_objc_members`` attribute maps to the ``@objcMembers`` Swift +attribute, which indicates that Swift members of this class, its subclasses, and +all extensions thereof, will impli

[PATCH] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

gribozavr2 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3387 +The ``swift_objc_members`` attribute maps to the ``@objcMembers`` Swift +attribute, which indicates that Swift members of this class, its subclasses, and +all extensions thereof, will impli

[PATCH] D90540: [Syntax] Add minimal TableGen for syntax nodes. NFC

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Syntax.td:9 +// +// The C++ grammatical structure modeled by libSyntax is quite regular. +// ==

[PATCH] D90540: [Syntax] Add minimal TableGen for syntax nodes. NFC

gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Syntax.td:31 + +// NodeType is the root of the archetype hierarchy. +class NodeType { "Records derived from NodeType correspond to syntax tree node types." Repository: rG LLVM Gi

[PATCH] D90543: [Syntax] Start to move trivial Node class definitions to TableGen. NFC

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. > e.g. we may introduce abstract bases like "loop" that the grammar doesn't > care about in order to model is-a concepts that might make refactorings more > expressive. This is less na

[PATCH] D90659: [Syntax] Tablegen Sequence classes. NFC

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.td:123 +Role<"CloseParen", Token<"r_paren">>, + ]; +} The reason why in my prototype th

[PATCH] D91601: [clang-tidy] Fix an abseil-redundant-strcat-calls crash on 0-parameter StrCat().

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:51 + if (Call->getNumArgs() == 0) +return; // Remove 'Foo('

<    3   4   5   6   7   8   9   10   11   12   >