[PATCH] D58710: Added more detailed documentation for clangd

gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. The text was written mostly by Sam McCall, screenshots are mostly made by me. Repository:

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr updated this revision to Diff 188521. gribozavr marked 2 inline comments as done. gribozavr added a comment. Addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58710/new/ https://reviews.llvm.org/D58710 Files: clan

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr marked 4 inline comments as done. gribozavr added inline comments. Comment at: clang-tools-extra/docs/clangd/Installation.rst:360 + +- Pass an experimental `-background-index` command line argument. With + this feature enabled, clangd incrementally builds an index of

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tools-extra/docs/clangd/Installation.rst:360 + +- Pass an experimental `-background-index` command line argument. With + this feature enabled, clangd incrementally builds an index of

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr updated this revision to Diff 188527. gribozavr added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58710/new/ https://reviews.llvm.org/D58710 Files: clang-tools-extra/docs/clangd/ApplyClangTidyFixInVSCode.gif clang-tool

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr updated this revision to Diff 188531. gribozavr added a comment. Herald added a reviewer: serge-sans-paille. Added custom CSS for the details tag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58710/new/ https://reviews.llvm.org/D58710 F

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr updated this revision to Diff 188534. gribozavr added a comment. Separated user and developer documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58710/new/ https://reviews.llvm.org/D58710 Files: clang-tools-extra/docs/_static/

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr updated this revision to Diff 188535. gribozavr added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58710/new/ https://reviews.llvm.org/D58710 Files: clang-tools-extra/docs/_static/clang-tools-extra-styles.css clang-tool

[PATCH] D58710: Added more detailed documentation for clangd

gribozavr updated this revision to Diff 188536. gribozavr added a comment. Removed mentions of clangd v9. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58710/new/ https://reviews.llvm.org/D58710 Files: clang-tools-extra/docs/_static/clang-tools-

[PATCH] D58710: Added more detailed documentation for clangd

This revision was automatically updated to reflect the committed changes. Closed by commit rL354992: Added more detailed documentation for clangd (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D58717: Added documentation for clangd v9+ features

gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accep

[PATCH] D58717: Added documentation for clangd v9+ features

This revision was automatically updated to reflect the committed changes. Closed by commit rL354994: Added documentation for clangd v9+ features (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D58721: Added release notes for clangd 8

gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58721 Files: clang-tools-e

[PATCH] D58721: Added release notes for clangd 8

gribozavr updated this revision to Diff 188555. gribozavr added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58721/new/ https://reviews.llvm.org/D58721 Files: clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clangd/

[PATCH] D58768: Moved SymbolLocation into its own header and implementation file

gribozavr created this revision. gribozavr added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, mgorny. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58768 Files: clang-tools-extra/clangd/CMakeLists.txt

[PATCH] D58769: Moved DenseMap support for SymbolID into SymbolID.h

gribozavr created this revision. gribozavr added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58769 Files: clang-tools-extra/clangd/index/Index.h clang-to

[PATCH] D58768: Moved SymbolLocation into its own header and implementation file

This revision was automatically updated to reflect the committed changes. Closed by commit rL355082: Moved SymbolLocation into its own header and implementation file (authored by gribozavr, committed by ). Herald added subscribers: llvm-commits, ilya-biryukov. Herald added a project: LLVM. Change

[PATCH] D58769: Moved DenseMap support for SymbolID into SymbolID.h

gribozavr closed this revision. gribozavr added a comment. Committed as https://reviews.llvm.org/rL355081. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58769/new/ https://reviews.llvm.org/D58769 ___ c

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

gribozavr created this revision. gribozavr added a reviewer: ioeric. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, mgorny. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58773 Files: clang-tools-extra/clangd/CMak

[PATCH] D58774: Moved Symbol into its own header and implementation file

gribozavr created this revision. gribozavr added a reviewer: ioeric. Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, mgorny. Herald added a project: clang. gribozavr added a parent revision: D58773: Moved SymbolOrigin into its own header and implementation file. Re

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

gribozavr added a comment. SymbolOrigin is used by itself, for example, in CodeComplete.h to define `struct CodeCompletion`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58773/new/ https://reviews.llvm.org/D58773 __

[PATCH] D58773: Moved SymbolOrigin into its own header and implementation file

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE355086: Moved SymbolOrigin into its own header and implementation file (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58773?vs=188703&id=188709#toc Reposit

[PATCH] D58774: Moved Symbol into its own header and implementation file

gribozavr updated this revision to Diff 188712. gribozavr added a comment. Moving SymbolSlab as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58774/new/ https://reviews.llvm.org/D58774 Files: clang-tools-extra/clangd/CMakeLists.txt clang

[PATCH] D58774: Moved Symbol into its own header and implementation file

gribozavr added a comment. Moved SymbolSlab as well, PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58774/new/ https://reviews.llvm.org/D58774 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D58774: Moved Symbol into its own header and implementation file

gribozavr updated this revision to Diff 188714. gribozavr added a comment. Herald added a subscriber: mgrang. Also moved the SymbolSlab implementation into Symbol.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58774/new/ https://reviews.llvm.org

[PATCH] D58774: Moved Symbol into its own header and implementation file

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE355088: Moved Symbol into its own header and implementation file (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58774?vs=188714&id=188715#toc Repository:

[PATCH] D58778: Moved Ref into its own header and implementation file

gribozavr created this revision. gribozavr added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous, mgorny. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58778 Files: clang-tools-extra/clangd/AST.h

[PATCH] D58778: Moved Ref into its own header and implementation file

gribozavr updated this revision to Diff 188723. gribozavr marked an inline comment as done. gribozavr added a comment. Added a license header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58778/new/ https://reviews.llvm.org/D58778 Files: clang-

[PATCH] D58778: Moved Ref into its own header and implementation file

gribozavr updated this revision to Diff 188724. gribozavr marked 2 inline comments as done. gribozavr added a comment. Added a missing include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58778/new/ https://reviews.llvm.org/D58778 Files: clang

[PATCH] D58778: Moved Ref into its own header and implementation file

gribozavr marked 2 inline comments as done. gribozavr added inline comments. Comment at: clang-tools-extra/clangd/index/Ref.cpp:1 +#include "Ref.h" + ioeric wrote: > License? Added. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp

[PATCH] D58781: Added missing license headers

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

[PATCH] D58778: Moved Ref into its own header and implementation file

This revision was automatically updated to reflect the committed changes. gribozavr marked an inline comment as done. Closed by commit rL355090: Moved Ref into its own header and implementation file (authored by gribozavr, committed by ). Herald added subscribers: llvm-commits, ilya-biryukov. Hera

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

gribozavr created this revision. gribozavr added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58782 Files: clang-tools-extra/clangd/index/Ref.cpp Index: c

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE355091: Use ArrayRef::copy, instead of copying data manually (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58782?vs=188729&id=188730#toc Repository: rCT

[PATCH] D58781: Added missing license headers

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE355092: Added missing license headers (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D58781?vs=188726&id=188731#toc Repository: rCTE Clang Tools Extra CH

[PATCH] D58814: [clang][Index] Constructors and Destructors do not reference class

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Please also add a test for find references on a constructor. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58814/new/ https://reviews.llvm.org/D58814

[PATCH] D58815: [clangd] Make sure constructors do not reference class

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: unittests/clangd/XRefsTests.cpp:1361 +} + )cpp", }; Add the new test right after the "Method call" test, to keep it gro

[PATCH] D58814: [clang][Index] Constructors and Destructors do not reference class

gribozavr added a comment. > These references were added to support using the index data for symbol rename. Understood -- thank you for providing the context (the original commit that added this code didn't have the explanation why this reference was added). However, my suggestion would be to

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

gribozavr added a comment. > The test coverage i have will be in the clang-tidy check. Please add unit tests to `clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp`. You can pull matchers from D57113 into the shared matchers library (instead of keeping them p

[PATCH] D39050: Add index-while-building support to Clang

gribozavr added inline comments. Herald added a subscriber: jdoerfert. Comment at: include/clang/Frontend/FrontendOptions.h:377 + /// The path to write index data to + std::string IndexStorePath; Please end comments with a period. Comment a

[PATCH] D58979: [clang][OpenMP] Revert "OMPFlushClause is synthetic, no such clause exists"

gribozavr accepted this revision. gribozavr added a comment. > Hmm, do we really need the matches for the AST node that is not > described/defined by the standard? If this is really so, I'm ok with this. When the rubber meets the road, it does not matter what is in the spec, users will need to

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

gribozavr added a comment. > My understanding is that the storage space taken up for Refs is currently 8 > bytes per Ref (4 each for the start and end positions), plus filename strings > which are deduplicated across all refs. If we add a SymbolID, that adds an > additional 8 bytes to each Ref.

[PATCH] D59083: [clangd] Store explicit template specializations in index for code navigation purposes

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CodeComplete.cpp:1513 +template bool explicitTemplateSpecialization(const NamedDecl &ND) { + if (const auto *TD = dyn_cast(&ND))

[PATCH] D58814: [clang][Index] Constructors and Destructors do not reference class

gribozavr accepted this revision. gribozavr added a comment. Do we also need to change anything for constructor calls? If not, please add a test for that. Comment at: lib/Index/IndexDecl.cpp:251 + Ctor->getParent(), Ctor->getDeclContext(), +

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clangd/GlobalCompilationDatabase.cpp:24 llvm::StringRef ResourceDir) { + // Clangd does not generate dependency file. + Cmd.Comm

[PATCH] D59086: [clangd] Adjust compile commands to be applicable for tooling

gribozavr added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:24 llvm::StringRef ResourceDir) { + // Clangd does not generate dependency file. + Cmd.CommandLine = tooling::getClangStripDependencyFileAdjuster()( kadircet

[PATCH] D59083: [clangd] Store explicit template specializations in index for code navigation purposes

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clangd/CodeComplete.cpp:1613 }; + // We only complete symbol's name, which is same as the class template in the + // case of template specializations. which is the same as the nam

[PATCH] D58815: [clangd] Make sure constructors do not reference class

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clangd/XRefs.cpp:160 +if (Roles & static_cast(index::SymbolRole::NameReference)) + return true; if (Loc == SearchedLocation) { Maybe add a blank line so that it is clear

[PATCH] D59128: [clangd] Redirect clangd page.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Fancy! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59128/new/ https://reviews.llvm.org/D59128

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:266 + + /// Returns the AST statement representing OpenMP structured-block of this + /// OpenMP executable directive, "the AST node" Comment at: include/clang/AST/Stm

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:269 + /// or if this is an OpenMP stand-alone directive returns `None`. + llvm::Optional getStructuredBlock() const; }; Why not `Stmt *` as a return value? Repository: rC Clang CH

[PATCH] D58749: [index-while-building] IndexRecordHasher

gribozavr added a comment. I left some comments, but it is difficult for me to review without understanding what the requirements for this hasher are, why some information is hashed in, and some is left out, could you clarify? See detailed comments. > This implementation is covered by lit test

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: include/clang/AST/StmtOpenMP.h:269 + /// or if this is an OpenMP stand-alone directive returns `None`. + llvm::Optional getStructuredBlock() const; }; lebedev.ri wrote: > gribozavr wrote: > > Why not `Stmt *` as a r

[PATCH] D58749: [index-while-building] IndexRecordHasher

gribozavr added a comment. > I basically didn't really like the idea of testing against hard-coded hash > values in unittests as I consider it to be an implementation detail. Sorry, that's not what I was suggesting. There are better ways to test hashing. For example, write a piece of source c

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: test/AST/ast-dump-openmp-atomic.c:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -ast-dump %s | FileCheck -strict-whitespace -implicit-check-not=openmp_structured_block %s + lebedev.ri wrote: > gribozav

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

gribozavr added a comment. Sorry for the delay. I will finish reviewing tomorrow. Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:9 +/// \file +/// \brief Utility class for listening for file system changes in a directory. +//===

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp:57 // Find all TypeLocs. The relevant Style Guide rule only applies to C++. - if (!getLang

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp:57 // Find all TypeLocs. The relevant Style Guide rule only applies to C++. - if (!getLangOpts().CPlusPlus) + // This check is a

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

gribozavr requested changes to this revision. gribozavr added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:27 +/// be invoked even if the directory is empty. +class DirectoryWatcher { +public:

[PATCH] D59407: [clangd] Add RelationSlab

gribozavr added inline comments. Comment at: clang-tools-extra/clangd/index/Index.cpp:35 +auto *Array = Arena.Allocate(Rels.size()); +std::uninitialized_copy(Rels.begin(), Rels.end(), Array); +Result.emplace_back(Entry.first, Use `ArrayRef::copy()`, f

[PATCH] D59407: [clangd] Add RelationSlab

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/Index.h:59 +return sizeof(*this) + Arena.getTotalMemory() + + sizeof(value_type) * Relations.size(); + } -

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

gribozavr added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6390 +/// Given OpenMP directive, matches the first clause (out of all specified), +/// that matches InnerMatcher. +/// Other comments usually don't explain the interactions with

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

gribozavr added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said matcher, + // and convert it to the OpenMPClau

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

gribozavr added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6389 +/// Given OpenMP directive, matches if it is an OpenMP standalone directive, +/// i.e. an executable directive with no structured block. "Matches standalone OpenMP direct

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

gribozavr added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said matcher, + // and convert it to the OpenMPClau

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: include/clang/AST/Stmt.h:102 +/// This bit is set only for the Stmt's that are the structured-block +/// of OpeMP [non-standalone] executable directives. +/// I.e. those returned by OMPExecutableDirective::getStructuredBloc

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

gribozavr added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said matcher, + // and convert it to the OpenMPClau

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added a comment. As far as I'm concerned, I don't have any major revision requests. I haven't reviewed the unit tests (I'm planning to), but for non-test code, I don't have any further comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59214/new/

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64 +++i; +})"; + ASSERT_TRUE( Same comment as in the other patch -- I would prefer that the source is inlined into the ASSERT_TRUE, with implicit string concatenation and "\n"

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr added inline comments. Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64 +++i; +})"; + ASSERT_TRUE( lebedev.ri wrote: > gribozavr wrote: > > Same comment as in the other patch -- I would prefer that the source is > > inlined into the ASSERT_TRUE

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/AST/Stmt.h:102 +/// This bit is set only for the Stmt's that are the structured-block +/// of OpeMP [non-standalone] executable di

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.h:156 + virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) {} + Please document that

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h:32 + +/// \brief Handles PPCallbacks and replays preprocessing with modules disabled. +/// replays => re-runs Repository: rG LLVM Github Monorepo CHAN

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

gribozavr added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:208 +template <> +void ExceptionAnalyzer::analyze(const FunctionDecl *Func, +ExceptionInfo &ExceptionList) { I'd suggest to make it a non-template

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53 + Finder->addMatcher(ompExecutableDirective( + unless(isStandaloneDirective()), + hasStructuredB

[PATCH] D59407: [clangd] Add RelationSlab

gribozavr added inline comments. Comment at: clang-tools-extra/clangd/index/Relation.h:1 +//===--- Ref.h ---*- C++-*-===// +// gribozavr wrote: > "--- Relation.h " Not done? C

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

gribozavr accepted this revision. gribozavr marked an inline comment as done. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:166 +Preprocessor->Lex(CurrentToken); +

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

gribozavr accepted this revision. gribozavr marked an inline comment as done. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53 + Finder->addMatcher(ompExecutableDirective( +

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

gribozavr accepted this revision. gribozavr marked an inline comment as done. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoil

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226 +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyzeBoilerplate(const T *Node) { + ExceptionInfo ExceptionList; lebedev.r

[PATCH] D59407: [clangd] Add RelationSlab

gribozavr added a comment. Submitting code as it becomes ready is the usual practice here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59407/new/ https://reviews.llvm.org/D59407 ___ cfe-commits maili

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: llvm/include/llvm/Testing/Support/Annotations.h:9 // // Annotations lets you mark points and ranges inside source code, for tests: // ---

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:69 + /// The tokens after preprocessor replacements. + llvm::ArrayRef tokens(const TokenBuffer &B) const; + /// Tokens that appear in the text of the file, i.e. a name of an object-li

[PATCH] D60040: [clangd] Use capacity() instead of size() in RefSlab::bytes()

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Do you have commit access? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60040/new/ https://reviews.llvm.org/D60040 ___

[PATCH] D60055: Check i < FD->getNumParams() before querying

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. This fix makes sense to me, however, Richard Smith @rsmith is the best person to review this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

gribozavr added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:756 /// lazily. mutable llvm::DenseMap RedeclComments; The name of this variable (and `RawCommentAndCacheFlags`) don't make sense after the change. Comme

[PATCH] D60055: Check i < FD->getNumParams() before querying

gribozavr added a comment. I'd be happy to land it, but I do want @rsmith to take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60055/new/ https://reviews.llvm.org/D60055 ___ cfe-commits mailin

[PATCH] D60055: Check i < FD->getNumParams() before querying

This revision was automatically updated to reflect the committed changes. Closed by commit rC358134: Check i < FD->getNumParams() before querying (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D60055?vs=194152&id=194581#toc Repository: rC Clang CHA

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

gribozavr added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:728 - class RawCommentAndCacheFlags { + class CommentAndOrigin { public: `RawCommentAndOrigin`? Comment at: clang/include/clang/AST/ASTContext.h:751 - /

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

gribozavr added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst:10 +functions are marked with ``__attribute__((warn_unused_result))``, but +the compiler warning for this attribute is not always enabled. + IIRC

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

gribozavr added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst:10 +functions are marked with ``__attribute__((warn_unused_result))``, but +the compiler warning for this attribute is not always enabled. + tmroe

[PATCH] D58573: [analyzer] Move UninitializedObjectChecker out of alpha

gribozavr added a comment. Thanks for the checker! We are hitting a crash on `_Atomic` fields, could you take a look? https://bugs.llvm.org/show_bug.cgi?id=41590 Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58573/new/ https://reviews.llvm.org/D58573

[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

gribozavr accepted this revision. gribozavr added a comment. Thanks for the fix! Comment at: test/Analysis/cxx-uninitialized-object.cpp:1145 + +void entry() { + MyAtomicInt b; `c11atomicTest()` Repository: rC Clang CHANGES SINCE LAST ACTION https://revi

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

gribozavr added a comment. > The FIXME in tests is fixed now. ... so instead of deleting the test, could you change it to show the current, better diagnostic? Comment at: clang/include/clang/AST/ASTContext.h:818 + /// For every comment not attached to any decl check if it sh

[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

gribozavr added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:800 + /// The result doesn't contain decls that don't have any comment attached. + std::unordered_map getRawCommentsForDeclsNoCache( + const std::unordered_map> Why not Dense

[PATCH] D58573: [analyzer] Move UninitializedObjectChecker out of alpha

gribozavr added a comment. Thank you for the fix! Another crash: https://bugs.llvm.org/show_bug.cgi?id=41611 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58573/new/ https://reviews.llvm.org/D58573 ___ cfe-commits

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

gribozavr added a comment. Thank you for the quick fix! Comment at: test/Analysis/cxx-uninitialized-object.cpp:1155 +void __vector_size__LongTest() { + VectorSizeLong v; +} Could you also add tests where the vector member is being subscripted? for example, `

[PATCH] D61246: [analyzer][UninitializedObjectChecker] PR41611: Regard vector types as primitive

gribozavr added inline comments. Comment at: test/Analysis/cxx-uninitialized-object.cpp:1155 +void __vector_size__LongTest() { + VectorSizeLong v; +} gribozavr wrote: > gribozavr wrote: > > Could you also add tests where the vector member is being subscripted?

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

gribozavr added a comment. > Also, IIUC the test case that I deleted wasn't actually supposed to produce > any diagnostics and the fact that it did was a bug. We could keep it as a > regression test but I think it has a rather low value. WDYT? What do you mean? The issue that the test is tryin

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

gribozavr added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:584 +// terminating early. +for (auto CIt = RawComments.begin(); CIt != RawComments.end(); ++CIt) { + RawComment *C = *CIt; jkorous wrote: > gribozavr wrote: > > Scanning all c

  1   2   3   4   5   6   7   8   9   10   >