[PATCH] D103393: [clangd] Bump recommended gRPC version (1.33.2 -> 1.36.3)

kadircet accepted this revision. kadircet added a comment. looks like the https://lab.llvm.org/buildbot/#/builders/131 is green after the version bump. thanks for taking care of this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103393/new/ https

[PATCH] D103797: [clang] Use resolved path for headers in decluse diagnostics

kadircet updated this revision to Diff 350288. kadircet added a comment. - handle windows paths in tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103797/new/ https://reviews.llvm.org/D103797 Files: clang/lib/Lex/ModuleMap.cpp clang/test/M

[PATCH] D103797: [clang] Use resolved path for headers in decluse diagnostics

kadircet added a comment. It is the absolute path of the header. e.g: `module TestTU does not depend on a module exporting '/work/llvm/clang-tools-extra/clangd/AST.h'` rather than `module TestTU does not depend on a module exporting 'AST.h'` Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D103825: [clang] Do not crash when ArgTy is null in CheckArgAlignment

kadircet added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4567 if (ParamTy->isPointerType()) ArgTy = ArgTy->getPointeeType(); i think ArgTy becomes null after this operation. e.g. the function requires a pointer param, user provides a

[PATCH] D103825: [clang] Do not crash when ArgTy is null in CheckArgAlignment

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. as discussed offline LG from my side. regarding the default arg of a parameter containing an error, it seems to be effecting type information of the parameter even though the type is expli

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

kadircet updated this revision to Diff 352628. kadircet marked 2 inline comments as done. kadircet added a comment. - Bail out early before filling in diag info - Move isExcluded check into handleDiagnostics, rather than handling it during flushing Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

This revision was automatically updated to reflect the committed changes. Closed by commit rGb662651586be: [clangd] Use command line adjusters for inserting compile flags (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99523/new

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG204014ec7557: [clangd] Fix feature modules to drop diagnostics (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D104540: [clangd] Dont index ObjCCategoryDecls for completion

kadircet created this revision. kadircet added a reviewer: dgoldman. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. They are already provided by Sem

[PATCH] D104540: [clangd] Dont index ObjCCategoryDecls for completion

kadircet updated this revision to Diff 353658. kadircet marked an inline comment as done. kadircet added a comment. - Also handle ObjcCategoryImplDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104540/new/ https://reviews.llvm.org/D104540 Files

[PATCH] D104540: [clangd] Dont index ObjCCategoryDecls for completion

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1918 + // from the index, we reduce the noise in all the other completion scopes. + if (llvm::isa(&ND)) +return false; dgoldman wrote: > Seems like we should also ignore O

[PATCH] D104540: [clangd] Dont index ObjCCategoryDecls for completion

This revision was automatically updated to reflect the committed changes. Closed by commit rG544d20eab662: [clangd] Dont index ObjCCategoryDecls for completion (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104540/new/ https:/

[PATCH] D104117: [clangd] Fix highlighting for implicit ObjC property refs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:579 + return true; +// A single property expr can reference both a getter and setter, but we can +// only provide a single semantic token, so prefer the getter. ---

[PATCH] D104841: [clangd] Call malloc_trim in clangd-index-server periodically

kadircet created this revision. kadircet added a reviewer: kbobyrev. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Mon

[PATCH] D104843: [clangd] Introduce a log-prefix flag to remote-index-server

kadircet created this revision. kadircet added a reviewer: kbobyrev. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Mon

[PATCH] D107040: [clangd] Make use of diagnostic tags for some clang diags

kadircet updated this revision to Diff 363038. kadircet added a comment. - Fix base Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107040/new/ https://reviews.llvm.org/D107040 Files: clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/cl

[PATCH] D107130: [clangd] Enable relative configs in check mode

kadircet updated this revision to Diff 363040. kadircet marked an inline comment as done. kadircet added a comment. - Don't use local configs for temp files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107130/new/ https://reviews.llvm.org/D107130

[PATCH] D106934: [clangd] Do not inlay hints pertaining to code in other files

kadircet added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:320 +// file that was included after the preamble), do not show in that case. +if (AST.getSourceManager().getFileID(FileRange->getBegin()) != MainFileID) + return; sa

[PATCH] D107130: [clangd] Enable relative configs in check mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc3682a22c227: [clangd] Enable relative configs in check mode (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D107040: [clangd] Make use of diagnostic tags for some clang diags

kadircet updated this revision to Diff 363053. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107040/new/ https://reviews.llvm.org/D107040 Files: clang-tools-

[PATCH] D107040: [clangd] Make use of diagnostic tags for some clang diags

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG57346526c83e: [clangd] Make use of diagnostic tags for some clang diags (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D106669: [clangd] Unify compiler invocation creation

kadircet updated this revision to Diff 363055. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106669/new/ https://reviews.llvm.org/D106669 Files: clang-tools-

[PATCH] D106669: [clangd] Unify compiler invocation creation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG41e24222861f: [clangd] Unify compiler invocation creation (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D106796: [clangd] Record remote index usage

This revision was automatically updated to reflect the committed changes. Closed by commit rG8070bf8c6e6a: [clangd] Record remote index usage (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106796/new/ https://reviews.llvm.org/

[PATCH] D107047: [clangd] Fix the crash in getQualification

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107047/new/ https://reviews.llvm.org/D107047 __

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

kadircet added a comment. I am not sure this is a good idea. Surely we can go with this approach and pretend to have the same GNUC version as the toolchain, but we are still using clang underneath as @joerg pointed out and we might not be able to parse language/compiler extensions supported by

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:682 + // here since editors won't properly render the symbol otherwise. + StringRef MaybeGroupName = Name; + if (MaybeGroupName.consume_front("-") && dgoldman wrote: > kadirce

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

kadircet added a comment. > The most confusing thing for me that we already have this implicitly set > -fgnuc-version=4.2.1. So, we already have implicitly set __GNUC__ and other > GCC macros as a default behavior. It might be because clang actually supports all the GNU extensions up until 4.2

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

kadircet added a comment. Sorry I think I've failed to explain my point. My concern is not about changing the way clang is parsing the code. I am saying that people might make use of compiler extensions by guarding their code with GNUC macros. Today we know that having 4.2.1 for GNUC version in

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

kadircet added a comment. In D107304#2922806 , @ArcsinX wrote: > @kadircet Thanks for clarification. Now your position is clear for me. > > But clang was designed as a drop-in replacement for GCC: > > - https://clang.llvm.org/docs/LanguageExtensions.html#

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

kadircet added a comment. Ok if you think that setting GCC version to higher values by default won't break anything, I think we should update this for all of clang instead. Apparently that was something done in the past already in https://github.com/llvm/llvm-project/commit/b5fe494a2cdd33a0b069

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:664 + } + // NextPragma is after us but before the next symbol. Our reign is over. + break; I suppose this is not correct in cases like. ``` #pragma mark - Outer

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

kadircet added a comment. Thanks! I think it looks good, I've suggested some more simplifications to termination detection. If you can delete the rest I'd like to take a final look at the rest of the details. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:680 +// We

[PATCH] D107624: [clangd] Rename Features.h -> Feature.h to avoid confilct with libstdc++

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

[PATCH] D107632: [clangd] Avoid "expected one compiler job" by picking the first eligible job.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CompilerTests.cpp:59 + +TEST(BuildCompilerInvocation, MultiArch) { + TestTU TU = TestTU::withHeaderCode(R"cpp( maybe move this test to `clang/unittests/Frontend/ASTUnitTest.cpp`? some

[PATCH] D107634: [clangd] Strip mutliple arch options

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, mgrang. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This patch strips all t

[PATCH] D96324: [clangd] Rename references to function arguments within the same file

kadircet added a comment. I think this is a great use case for pseudo parser actually (unfortunately requires some multi-file interactions). In theory we can just fetch all the references, pseudo-parse the files, and update all the declarations & definitions to have the same parameter name. My

[PATCH] D107637: [clangd] Canonicalize inputs provided with `--`

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. We already strip all the inputs

[PATCH] D107634: [clangd] Strip mutliple arch options

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:225 + llvm::SmallVector IndicesToDrop; + // Having multiple architecture options (e.g. when building FAT binaries) sammccall wr

[PATCH] D107634: [clangd] Strip mutliple arch options

kadircet updated this revision to Diff 364763. kadircet marked an inline comment as done. kadircet added a comment. - Update comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107634/new/ https://reviews.llvm.org/D107634 Files: clang-tools-e

[PATCH] D107637: [clangd] Canonicalize inputs provided with `--`

kadircet updated this revision to Diff 364764. kadircet marked an inline comment as done. kadircet added a comment. - Cmd.resize instead of dropping arguments trailing `--` one by one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107637/new/ https

[PATCH] D107634: [clangd] Strip mutliple arch options

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3bf77980d934: [clangd] Strip mutliple arch options (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D107637: [clangd] Canonicalize inputs provided with `--`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG79c2616d315f: [clangd] Canonicalize inputs provided with `--` (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D107637: [clangd] Canonicalize inputs provided with `--`

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:250 + ArgList.getLastArgNoClaim(driver::options::OPT__DASH_DASH)) { +for (auto I = 0U; I <= DashDash->getNumValues(); ++I) + IndicesToDrop.push_back(DashDash->getIndex() + I

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

kadircet added a comment. thanks, mostly looks good couple of nits! Comment at: clang-tools-extra/clangd/CollectMacros.cpp:42 +if (isInsideMainFile(Loc, SM)) { + Position Start = sourceLocToPosition(SM, Loc); + Position End = {Start.line + 1, 0};

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/ParsedAST.cpp:441 + if (Preamble) +Marks = Preamble->Marks; + Clang->getPreprocessor().addPPCallbacks( --

[PATCH] D107632: [clangd] Avoid "expected one compiler job" by picking the first eligible job.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/Compiler.cpp:42 const clang::Diagnostic &Info) { + DiagnosticConsumer::Handl

[PATCH] D107703: [AST][clangd] Expose documentation of Attrs on hover.

kadircet accepted this revision. kadircet added a comment. thanks, LG from my side as well, just a couple of clarification questions regarding tablegen. Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:2389 + HI.Documentation = Attr::getDocumentation(attr:

[PATCH] D104841: [clangd] Call malloc_trim in clangd-index-server periodically

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3aa6ca8def51: [clangd] Call malloc_trim in clangd-index-server periodically (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D104843: [clangd] Introduce a log-prefix flag to remote-index-server

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8f2bf93b5bd6: [clangd] Introduce a log-prefix flag to remote-index-server (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D105039: [clangd] Add a flag to disable formatting of tweak edits

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Some tweaks might edit file typ

[PATCH] D105039: [clangd] Add a flag to disable formatting of tweak edits

kadircet updated this revision to Diff 354941. kadircet added a comment. Add a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105039/new/ https://reviews.llvm.org/D105039 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/c

[PATCH] D105039: [clangd] Add a flag to disable formatting of tweak edits

This revision was automatically updated to reflect the committed changes. Closed by commit rG614b46e4dcab: [clangd] Add a flag to disable formatting of tweak edits (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105039/new/ htt

[PATCH] D105083: [clangd] Ensure Ref::Container refers to an indexed symbol

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:334 +const Decl *SymbolCollector::getRefContainer(const Decl *Enclosing) { + const NamedDecl *ND = dyn_cast_or_null(Enclosing); why do we need to expose this functio

[PATCH] D105083: [clangd] Ensure Ref::Container refers to an indexed symbol

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:159 +// indexed, we walk further up because Ref::Container should always be +// an indexed symbol. +const Decl *getRefContainer(const Decl *Enclosing, can you also add

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp:280 +#if CLANGD_TIDY_CHECKS TEST_F(ConfigCompileTests, Tidy) { why do we need to disable this test? it doesn't really build an ast or assert on the diagnosti

[PATCH] D105681: [clangd] Add platform triple (host & target) to version info

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! only printing the target when they differ makes a lot of sense! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105681/new/ https

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:259 +class T{$explicit[[]]$constructor[[T]](int a);}; + sammccall

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

kadircet added a comment. So first of all, it is not clear to me if this is a useful enough improvement to justify all the new code, but looking at your investment I'll assume this is really common in ObjC land and gonna be useful for developers working with ObjC. That's enough of a justificati

[PATCH] D111711: [clangd] IncludeCleaner: Don't consider the definition as usage for function forward declarations

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:42 + bool VisitFunctionDecl(FunctionDecl *FD) { +// Function definition will require redeclar

[PATCH] D112527: [clangd] Fix a hover crash on templated spaceship operator.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Hover.cpp:271 + DeclForComment = TSD->getSpecializedTemplate(); if (const auto *TIP = TSD->getTemplateInstantiationPatt

[PATCH] D112530: [clangd] AddUsing: Fix support for template specializations.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:284 +// Remove the template arguments from the name. +NameRange = SourceRange(Na

[PATCH] D112559: [clangd] Fix filename ranges while replaying preamble

kadircet created this revision. kadircet added reviewers: sammccall, nridge. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Clangd used first token

[PATCH] D112530: [clangd] AddUsing: Fix support for template specializations.

kadircet added a comment. (oops, i didn't notice i wasn't the reviewer :D) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112530/new/ https://reviews.llvm.org/D112530 ___ cfe-commits mailing list cfe-comm

[PATCH] D112559: [clangd] Fix filename ranges while replaying preamble

This revision was automatically updated to reflect the committed changes. Closed by commit rGe42f5d4b488e: [clangd] Fix filename ranges while replaying preamble (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112559/new/ https:

[PATCH] D112565: [clangd] Add integration test for crash handling

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112565/new/ https://reviews.llvm.org/D112565 __

[PATCH] D112608: [clangd] IncludeCleaner: Do not process locations in built-in files

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:137 +// Check if Loc is not written in a physical file. +if (FID.isInvalid() || SM.isWritten

[PATCH] D112712: [clangd] SelectionTree should prefer lexical declcontext

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This is important especially fo

[PATCH] D112712: [clangd] SelectionTree should prefer lexical declcontext

kadircet updated this revision to Diff 383008. kadircet edited the summary of this revision. kadircet added a comment. - Related https://github.com/clangd/clangd/issues/900. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112712/new/ https://reviews.

[PATCH] D112707: [clangd] IncludeCleaner: Be more conservative in marking RecordDecl usage

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:51 + Result.insert(Definition ? Definition->getLocation() + : RD->getMostRecentDecl()->getLocation()); + return true; i think this migh

[PATCH] D112712: [clangd] SelectionTree should prefer lexical declcontext

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:751 + { +auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), + Test.poi

[PATCH] D112712: [clangd] SelectionTree should prefer lexical declcontext

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG3d735480bd2a: [clangd] SelectionTree should prefer lexical declcontext (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D112712?vs=383

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:275 std::vector computeUnusedIncludes(ParsedAST &AST) { + trace::Span Tracer("IncludeCleaner::computeUnusedIncludes"); const auto &SM = AST.getSourceManager(); this doesn

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. oops forgot to LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112783/new/ https://reviews.llvm.org/D112783 ___

[PATCH] D112835: [clangd] Record time spent in tidy checks

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Mo

[PATCH] D113029: [clangd] Find definition of ClassTemplate without going through index.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113029/new/ https://reviews.llvm.org/D113029 __

[PATCH] D113120: [clang] Add early exit when checking for const init of arrays.

kadircet added a comment. thanks this looks amazing! i am also not an expert in these parts of the code but AFAICT the proposed behavior is in line with the contract. i am a little worried about the cost of extra copy (especially when there are only a handful of elements), but it should be prob

[PATCH] D113120: [clang] Add early exit when checking for const init of arrays.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/lib/AST/ExprConstant.cpp:10617 +// copying the data, which is wasteful. +for (const unsigned N : {1u, FinalSize}) { + unsigne

[PATCH] D113390: [clangd] Dont include file version in task name

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, javed.absar. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This will drop fil

[PATCH] D113555: [clangd] Mark macros from preamble for code completion

kadircet created this revision. kadircet added reviewers: usaxena95, nridge. Herald added a subscriber: arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. If the main file is a header, mark

[PATCH] D113645: [clangd] Allow Unix config paths on Darwin

kadircet added a subscriber: sammccall. kadircet added a comment. I got a couple of concerns about the general ideas in the change. Even though the convenience of using ~/.config/clangd/config.yaml seems nice, I think it'll just end up creating confusion in the long run. According to various ma

[PATCH] D113555: [clangd] Mark macros from preamble for code completion

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG7d668ae38d2d: [clangd] Mark macros from preamble for code completion (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D113555?vs=38610

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:476 Completion.Kind == CompletionItemKind::Method || Completion.Kind == CompletionItemKind::Constructor) { // If there is a potential template argument list, drop snippe

[PATCH] D97417: [clangd] use a compatible preamble for the first AST built

kadircet added a comment. In D97417#2588101 , @qchateau wrote: > Well indeed do some extra work if we elect a compatible but almost useless > preamble. We'll basically do the work twice (but at least we do it > concurrently \o/). I am afraid this can ha

[PATCH] D97536: [clangd][remote] Add flag to set idletimeout

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. By default gRPC has no idletimeout and some

[PATCH] D97536: [clangd][remote] Add flag to set idletimeout

kadircet updated this revision to Diff 326632. kadircet marked an inline comment as done. kadircet added a comment. Change 10 seconds to 600 seconds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97536/new/ https://reviews.llvm.org/D97536 Files:

[PATCH] D97536: [clangd][remote] Add flag to set idletimeout

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:84 +llvm::cl::opt IdleTimeoutSeconds( +"idle-timeout", llvm::cl::init(10), +llvm::cl::desc("Maximum time a channel may stay idle until server closes " samm

[PATCH] D97548: [clangd] Introduce client state invalidation

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Clangd can invalidate client state of featu

[PATCH] D97548: [clangd] Introduce client state invalidation

kadircet added inline comments. Comment at: clang-tools-extra/clangd/test/semantic-tokens-refresh.test:13 +# CHECK:"method": "workspace/semanticTokens/refresh", +# CHECK-NEXT: "params": null +# CHECK-NEXT: } lsp spec says `params: none` for this reques

[PATCH] D97536: [clangd][remote] Add flag to set idletimeout

kadircet updated this revision to Diff 326667. kadircet marked an inline comment as done. kadircet added a comment. - change default from 10 to 8 minutes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97536/new/ https://reviews.llvm.org/D97536 File

[PATCH] D97536: [clangd][remote] Add flag to set idletimeout

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1a5dfb7db23e: [clangd][remote] Add flag to set idletimeout (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97536: [clangd][remote] Add flag to set idletimeout

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:84 +llvm::cl::opt IdleTimeoutSeconds( +"idle-timeout", llvm::cl::init(10), +llvm::cl::desc("Maximum time a channel may stay idle until server closes " samm

[PATCH] D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics

kadircet updated this revision to Diff 326686. kadircet added a comment. - Update header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96245/new/ https://reviews.llvm.org/D96245 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-e

[PATCH] D97555: [clangd] Add diagnostic augmentation hook to Modules

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman, javed.absar. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D96245

[PATCH] D96439: [clangd][WIP] Introduce DiagnosticAugmentationModule

kadircet abandoned this revision. kadircet added a comment. see https://reviews.llvm.org/D97555 for revised version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96439/new/ https://reviews.llvm.org/D96439 __

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:279 SymbolSlabs.push_back(FileAndSymbols.second); + for (const auto &S : *FileAndSymbols.second) { +if (S.Definition) iterating over all the symbols here

[PATCH] D97615: [clangd] Include macro expansions in documentSymbol hierarchy

kadircet added a comment. thanks, LGTM. with a couple nits and asking for some extra clarifications :) Comment at: clang-tools-extra/clangd/FindSymbols.cpp:283 + public: +SymBuilder() = default; + nit: drop this one? Comment at: clang-to

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:292 +for (const auto &Sym : *Slab) { + if (Sym.Definition) +Files.insert(Sym.Definition.FileURI); it feels weird to choose one or the other here, why not jus

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:292 +for (const auto &Sym : *Slab) { + if (Sym.Definition) +Files.insert(Sym.Definition.FileURI); ArcsinX wrote: > kadircet wrote: > > it feels weird to choo

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

kadircet added a comment. Right, i see the problem you are trying to solve, and all of this is a mess working nicely in unisom, but falls apart if you try to isolate each piece :/ (hence the need for figuring out list of files while indexing instead) But until that day, I suppose the best we ca

<    19   20   21   22   23   24   25   26   27   28   >