[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

[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

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

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

[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

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

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

[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

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

[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

[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()`

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

[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

[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`.

[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

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

[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 , const ASTContext ) { + StringRef ExprText = getText(E, Context); + if (mayNeedParens(E)) ymandel wrote: >

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

[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

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

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

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

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

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

[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

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

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

[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] 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 ); -#include

[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

[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

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

[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

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

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

[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 ) { +

[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

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

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

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

[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

[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

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

[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

[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, ::FPR128_loRegClass); +case 'y': + if (!Subtarget->hasFPARMv8()) ```

[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

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

[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 ) { std::lock_guard MG(Mux); Set = ParsedRegions; jkorous wrote: > gribozavr wrote: > > I think we should lock both the source and

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

[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 ) const { +

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

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

[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

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

[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

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

[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

[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 = ...; ///< \return none CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66706/new/

[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 , + bool testTypedefTypeLoc = false) { TypeLoc PrevTL; Mordante wrote: > gribozavr wrote: > >

[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

[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

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:

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

[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

[PATCH] D66947: Changed FrontendActionFactory::create to return a std::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/D66947 Files: clang-tools-extra/clang-doc/ClangDoc.cpp

[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

gribozavr marked 2 inline comments as done. gribozavr added a comment. Committed as r370337. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66878/new/ https://reviews.llvm.org/D66878 ___ cfe-commits

[PATCH] D66879: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it instead of inventing their own solution

This revision was automatically updated to reflect the committed changes. Closed by commit rL370338: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored… (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D66877: Moved the IndexDataConsumer::finish call into the IndexASTConsumer from IndexAction

gribozavr closed this revision. gribozavr marked an inline comment as done. gribozavr added a comment. Committed as r370336. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66877/new/ https://reviews.llvm.org/D66877

[PATCH] D66876: Indexing: create PP callbacks in the ASTConsumer

gribozavr added a comment. Committed as r370323. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66876/new/ https://reviews.llvm.org/D66876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66875: [Index] Marked a bunch of classes 'final'

gribozavr added a comment. Committed as r370321. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66875/new/ https://reviews.llvm.org/D66875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66884: Removed dead code from clang/AST/NSAPI.h

This revision was automatically updated to reflect the committed changes. Closed by commit rL370298: Removed dead code from clang/AST/NSAPI.h (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66879: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it instead of inventing their own solution

gribozavr added inline comments. Comment at: clang/include/clang/Index/IndexingAction.h:60 + +inline std::unique_ptr createIndexingASTConsumer( +std::shared_ptr DataConsumer, jkorous wrote: > Why not use default argument instead of overloading? Writing a

[PATCH] D66884: Removed dead code from clang/AST/NSAPI.h

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66884 Files: clang/include/clang/AST/NSAPI.h clang/lib/AST/NSAPI.cpp Index: clang/lib/AST/NSAPI.cpp

[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

gribozavr marked 4 inline comments as done. gribozavr added inline comments. Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:217 std::unique_ptr Includes; + index::IndexingOptions Opts; std::unique_ptr PragmaHandler; ilya-biryukov wrote: > Are

[PATCH] D66877: Moved the IndexDataConsumer::finish call into the IndexASTConsumer from IndexAction

gribozavr marked 2 inline comments as done. gribozavr added inline comments. Comment at: clang/lib/Index/IndexingAction.cpp:77 +IndexCtx->getDataConsumer().setPreprocessor(PP); +PP->addPPCallbacks(std::make_unique(IndexCtx)); + } ilya-biryukov wrote: >

[PATCH] D66879: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it instead of inventing their own solution

gribozavr created this revision. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous. Herald added a project: clang. gribozavr added reviewers: ilya-biryukov, jkorous. Herald added a subscriber: dexonsmith. gribozavr added a parent revision: D66878: [Index] Stopped wrapping

[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

gribozavr created this revision. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous. Herald added a project: clang. gribozavr added reviewers: ilya-biryukov, jkorous. Herald added a subscriber: dexonsmith. gribozavr added a parent revision: D66877: Moved the

[PATCH] D66877: Moved the IndexDataConsumer::finish call into the IndexASTConsumer from IndexAction

gribozavr created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. Doing so removes the last reason to expose a FrontendAction from libIndex. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66877 Files:

[PATCH] D66875: [Index] Marked a bunch of classes 'final'

gribozavr created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. gribozavr added a reviewer: ilya-biryukov. gribozavr added a child revision: D66876: Indexing: create PP callbacks in the ASTConsumer. This file defines multiple inheritance

[PATCH] D66876: Indexing: create PP callbacks in the ASTConsumer

gribozavr created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. gribozavr added a parent revision: D66875: [Index] Marked a bunch of classes 'final'. Doing so removes one reason to create a custom FrontendAction. FrontendActions are not desirable

[PATCH] D66874: [clang-tidy] Fix the potential infinite loop in recordIsTriviallyDefaultConstructible.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/utils/TypeTraits.cpp:57 return true; + // Don't perform the check on an ill-formed Decl. As we will visit every

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70 +- New :doc:`bugprone-posix-return + ` check. jcai19 wrote: > Eugene.Zelenko wrote: > > Check is not new, just modified. Such check

[PATCH] D66797: ArrayRef'ized CompilerInvocation::CreateFromArgs

This revision was automatically updated to reflect the committed changes. Closed by commit rL370122: ArrayRefized CompilerInvocation::CreateFromArgs (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66797: ArrayRef'ized CompilerInvocation::CreateFromArgs

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr added a reviewer: ilya-biryukov. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66797 Files: clang/include/clang/Frontend/CompilerInvocation.h

[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

This revision was automatically updated to reflect the committed changes. Closed by commit rL370039: Refactor GlobList from an ad-hoc linked list to a vector (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

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