[PATCH] D79992: [WIP][clangd] Patch PP directives to use stale preambles while building ASTs

kadircet updated this revision to Diff 264666. kadircet added a comment. - MacroInfo's definition range is a token range, convert it to a char range. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79992/new/ https://reviews.llvm.org/D79992 Files:

[PATCH] D80160: [Tooling] Drop leading/trailing whitespace from compile_flags.txt lines

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks for remembering this :D Comment at: clang/test/Tooling/fixed-database.cpp:6 // RUN: cp "%S/Inputs/fixed-header.h" "%t/Include/" +// RUN: echo '# this is a comment

[PATCH] D80198: [clangd] locateMacroAt handles patched macros

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D79992 . Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D80198 Files: clang

[PATCH] D79992: [WIP][clangd] Patch PP directives to use stale preambles while building ASTs

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:419 + if (DirectivesChanged) { +// We need to patch all the directives, since they are order dependent. e.g: +// #define BAR(X) NEW(X) // Newly i

[PATCH] D80212: [clangd] Handle references for patched macros

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Depends on D80198 . Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D80212 Files: clang

[PATCH] D132830: [clangd] Avoid crash when printing call to string literal operator template in hover

kadircet added a comment. can you move the test into `llvm/llvm-project/clang/unittests/AST/StmtPrinterTest.cpp` ? while there, also a test on pack-extended user defined literals would be nice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132830/

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:202 FR.endLine = End.line; +FR.startCharacter = Start.character; FR.endCharacter = End.character; can you put it back to its previous location (i.e. right aft

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:254 +// Remove the ending sentinal "*/" from the block comment range. +if (Code.substr(EndOffset(*LastComment) - 2, 2) == "*/") { + End.character -= 2; this is

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

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/D131154/new/ https://reviews.llvm.org/D131154 __

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet created this revision. kadircet added reviewers: usaxena95, ilya-biryukov. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was showing up in our internal crash collector. I have no idea ho

[PATCH] D132919: [clangd] Enable folding ranges by default.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:340 desc("Enable preview of FoldingRanges feature"), -init(false), +init(true), Hidden, i think we should just retire the flag altogether, ATM this is only

[PATCH] D132919: [clangd] Enable folding ranges by default.

kadircet added a comment. In D132919#3757685 , @Trass3r wrote: > Whenever I tried this option in the past it crashed clangd. > I recommend doing some more testing before flipping the switch. we've a new implementation of folding ranges based on pseudo pa

[PATCH] D132919: [clangd] Enable folding ranges by default.

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/D132919/new/ https://reviews.llvm.org/D132919 __

[PATCH] D132918: [clang] Fix a crash in constant evaluation

This revision was automatically updated to reflect the committed changes. Closed by commit rGa5ab650714d0: [clang] Fix a crash in constant evaluation (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132918/new/ https://reviews.l

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet added a comment. sorry, i missed the other comments around having a lit test. reverting the change. Comment at: clang/lib/AST/ExprConstant.cpp:4797 } +// Can't access properties of an incomplete type. +if (!RD->hasDefinition()) { shafik w

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

kadircet added a comment. can you also add test cases for the other two (filtering both for speculative index queries/regular ones, and making sure we don't suggest symbols from index for category names), so that we don't regress in the future? Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3228 + + Results = completions(R"objc( + Fo^ nit: just `completions("Fo^", ...)` Comment at: clang-tools-extra/clangd/unittests/CodeCompl

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet updated this revision to Diff 457566. kadircet added a comment. Add reproducer. I think the issue is about keeping constexpr functions valid even when their bodies contain invalid decls under certain instantiations, which I believe is the right behaviour. As the function body might be in

[PATCH] D132830: [clangd] Avoid crash when printing call to string literal operator template in hover

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

[PATCH] D130863: [clangd] Make git ignore index directories

kadircet added a comment. In D130863#3696419 , @sums wrote: > Thank you for the reviews and discussion, everyone! > > TIL about `.git/info/exclude`. It'll let me exclude stuff without updating > the checked in `.gitignore`. I was achieving the same effec

[PATCH] D133339: [clangd] Isolate logic for setting LSPServer options

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Moves clangd

[PATCH] D130863: [clangd] Make git ignore index directories

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp:58 +if (CreatingNewDirectory) { + llvm::Error error = addGitignore(DiskShardRoot); + if (error) { nit: ``` if(llvm::Error E = addGitignore(..)) {

[PATCH] D133339: [clangd] Isolate logic for setting LSPServer options

kadircet updated this revision to Diff 458128. kadircet added a comment. rename helper & add comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19/new/ https://reviews.llvm.org/D19 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D132918: [clang] Fix a crash in constant evaluation

kadircet added a comment. > This is helpful information but I am not sure this convinces me that > EvaluateVarDecl is the correct place to check. Why not in EvaluateDecl or > EvaluateStmt? Both locations we could also check. I have done the check inside `EvaluateVarDecl` because the invalid eva

[PATCH] D133423: [clangd] Improve Selection testcase, pin to C++17

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. can you wait for premerge checks to finish (especially for windows)? Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:729-730 EXPECT_EQ("CXXConstructE

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1731 SymbolSlab::Builder ResultsBuilder; -if (Opts.Index->fuzzyFind(Req, [&](const Symbol &Sym) { -

[PATCH] D133479: [clangd] Set Incompleteness for spec fuzzyfind requests

kadircet created this revision. kadircet added reviewers: sammccall, dgoldman. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Rep

[PATCH] D130265: [clangd] resolve forwarded parameters in Hover

kadircet added a comment. This looks like a nice idea, but i think we definitely need annotations to prevent confusion. I'd prefer something like: assume `foo(int x, Args... args);` function foo -> void Parameters: - int x - int a : forwarded to [bar:a](it'd be great if we could have l

[PATCH] D133440: [clangd] Allow programmatically disabling rename of virtual method hierarchies.

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/refactor/Rename.cpp:217 return ReasonToReject::NonIndexable; + if (!Opts.RenameVirtual) { +if (const auto *S

[PATCH] D133339: [clangd] Isolate logic for setting LSPServer options

kadircet updated this revision to Diff 458769. kadircet added a comment. I agree this is creating confusing state for constructors of ClangdLSPServer. It would be interesting to create LSPServer after the initialize request one day. - Make client-capability related options private again. Reposi

[PATCH] D133479: [clangd] Set Incompleteness for spec fuzzyfind requests

This revision was automatically updated to reflect the committed changes. Closed by commit rG71c4fb1d6482: [clangd] Set Incompleteness for spec fuzzyfind requests (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D133479?vs=458690&id=458774#toc Repository: rG LLVM Git

[PATCH] D133757: [clangd] Turn QueryDriverDatabase into a CompileCommandsAdjuster

kadircet added a comment. thanks, i agree that we should make query driver work coherently with `CompileFlags.Compiler`, but I am not sure if we really need to complexity for handling this adjustment logic before resolving the driver. I've got some comments right next to the change but to summa

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1091 +// better, which in this example would be the actual declaration of foo. +auto *DeclToUse = Decls.begin(); +while (llvm::isa(*DeclToUse) && we actually w

[PATCH] D116443: [clangd] Implement textDocument/typeDefinition

kadircet added inline comments. Herald added a project: All. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1292 +for (const LocatedSymbol &Sym : *Types) + Response.push_back(Sym.PreferredDeclaration); +return Reply(std::move(Response)); ---

[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)

kadircet added inline comments. Herald added projects: clang-tools-extra, All. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1306 +for (const LocatedSymbol &Sym : *Overrides) + Impls.push_back(Sym.PreferredDeclaration); +return Reply(std::move(

[PATCH] D133843: [clangd] Prefer definitions for gototype and implementation

kadircet created this revision. kadircet added reviewers: sammccall, usaxena95. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Re

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1023 + // which in this example would be the actual declaration of foo. + if (Candidates.size() == 2) { +if (llvm::isa(Candidates.front())) nit: you can change this to `Candidate

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1011 +const NamedDecl * +pickDeclToUse(const llvm::SmallVector &Candidates) { + if (Candidates.empty()) you can just pass in ArrayRef instead. Repository: rG LLVM Github Monorepo

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! i think you have commit access now, but let me know if I should land this for you (preferably with an email address for commit attribution) Comment at: cla

[PATCH] D133843: [clangd] Prefer definitions for gototype and implementation

kadircet added a comment. In D133843#3791127 , @sammccall wrote: > There's no test here, can you describe the cases you expect this to affect > and why the new behavior is better? right, sorry for the obscure patch. giving examples/details in particular

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

kadircet added a comment. In D133664#3791694 , @tom-anders wrote: > Hmm, Github noticed that I referenced the issue with this commit, but didn't > close it. > According to > https://github.blog/2013-03-18-closing-issues-across-repositories/ closing >

[PATCH] D128462: [HLSL] add -I option for dxc mode.

kadircet added inline comments. Comment at: clang/include/clang/Driver/Options.td:6362 -def cl_Group : OptionGroup<"">, Flags<[CLOption]>, +def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>, HelpText<"CL.EXE COMPATIBILITY OPTIONS">; i am failing to unders

[PATCH] D133757: [clangd] Turn QueryDriverDatabase into a CompileCommandsAdjuster

kadircet added a comment. In D133757#3791485 , @nridge wrote: > I think the sticking point for just having `QueryDriverDatabase` run after > the entirety of `CommandMangler` is this check >

[PATCH] D128462: [HLSL] add -I option for dxc mode.

kadircet added a subscriber: jansvoboda11. kadircet added inline comments. Comment at: clang/include/clang/Driver/Options.td:6362 -def cl_Group : OptionGroup<"">, Flags<[CLOption]>, +def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>, HelpText<"CL.EXE COMPATIBILITY OPTIONS

[PATCH] D133962: [clang(d)] Include/Exclude CLDXC options properly

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added projects: clang, clang-tools-extra. This handles th

[PATCH] D128462: [HLSL] add -I option for dxc mode.

kadircet added a comment. BTW, I am not sure when the new DXC driver mode was introduced but ATM it isn't properly handled by clang-tooling APIs, especially clangd. As the logic there wasn't updated accordingly, and it also resulted in regressions for clang-cl mode. I've sent out https://review

[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:197 - "${" + - std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + ':'; appendEscapeSnippet(Chunk.Text, Snippet); as you've ment

[PATCH] D127856: [clangd] Support multiline semantic tokens

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG333620d37a26: [clangd] Support multiline semantic tokens (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129100: [clangd] Support external throttler for preamble builds

kadircet added a comment. we've already discussed most of these so no action needed, but here are some of them before they become completely irrelevant :P Comment at: clang-tools-extra/clangd/TUScheduler.cpp:457 +// Give up on this request, releasing resources if any. +

[PATCH] D129100: [clangd] Support external throttler for preamble builds

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/ClangdServer.h:108 +/// This throttler controls which preambles may be built at a given time. +clangd::Preamble

[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

kadircet added a comment. > A specific example i encountered is clang/Tooling/DiagnosticsYaml.h Which > defines template specializations for inputting/outputting yaml io. That file > must be included if you ever want to emit diagnostics as YAML, but the > typical use case is to just use the ope

[PATCH] D129100: [clangd] Support external throttler for preamble builds

kadircet added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:499 // Build the preamble and let the waiters know about it. build(std::move(*CurrentReq)); } regarding the flakiness, what about resetting the `Throttle

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

kadircet added a comment. The idea looks great in general, I didn't get a chance to look at all the details but this also creates some concerns for integrations of clang-tidy checks in other environments (like clangd or tidy running in distributed systems) as the workflow actually needs to be r

[PATCH] D122315: [clangd] Retain main file fixes attached to diags from preamble

kadircet updated this revision to Diff 417873. kadircet marked 2 inline comments as done. kadircet added a comment. - Get rid of redundant local variable - Update comment around assumptions about main file-ness inside toLSPDiag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122315: [clangd] Retain main file fixes attached to diags from preamble

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:68 return true; // Fixes are always in the main file. if (!D.Fixes.empty()) sammccall wrote: > update comment: Fixes are only added if the fix or diagnostics is in the

[PATCH] D122315: [clangd] Retain main file fixes attached to diags from preamble

This revision was automatically updated to reflect the committed changes. Closed by commit rG50f4f32b5668: [clangd] Retain main file fixes attached to diags from preamble (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122315/ne

[PATCH] D122894: [clangd] Record IO precentage for first preamble build of the instance

kadircet created this revision. kadircet added a reviewer: adamcz. Herald added subscribers: usaxena95, arphaman, javed.absar. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-

[PATCH] D122894: [clangd] Record IO precentage for first preamble build of the instance

kadircet updated this revision to Diff 419737. kadircet added a comment. - Fix c/p error.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122894/new/ https://reviews.llvm.org/D122894 Files: clang-tools-extra/clangd/TUScheduler.cpp Index: clang-

[PATCH] D122894: [clangd] Record IO precentage for first preamble build of the instance

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:115 bool IsFirstPreamble) { - static llvm::once_flag OnceFlag; - llvm::call_once(OnceFlag, [&] { + auto RecordWithLabel = [

[PATCH] D122894: [clangd] Record IO precentage for first preamble build of the instance

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rGe2f598bc1b37: [clangd] Record IO precentage for first preamble build of the instance (authored by kadircet

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kadircet added a comment. note that you seem to be using `auto PublicHeader = CanonIncludes.mapHeader(Entry->getName());` (i.e. `getName`) in D120306 and not `tryGetRealPathName`. it might be nice to have a test case here, at least one that only fails/runs on

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kadircet added a comment. > Right! I think I also need to do that in SymbolCollector.cpp. Now that you mentioned this too, it feels like the issue is not about being consistent, as I think all of the places that we have today are actually making use of `FileEntry::getName` as keys to `Canonica

[PATCH] D123144: FileManager: std::map => BumpPtrAllocator + DenseMap of pointers

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! maybe put an NFC in the patch name? Comment at: clang/lib/Basic/FileManager.cpp:581 /// do directory look-up instead of file look-up. -std::error_code -FileManag

[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks lgtm! I would still be looking out for build bot statuses in case there are OS specific code paths that were using isvalid. it might also be worthwhile to send out `operator<` chang

[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

kadircet added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:462 UFE->Dir = &DirInfo->getDirEntry(); - UFE->UID = NextFileUID++; - UFE->IsValid = true; + UFE->UID = NextFileUID++; UFE->File.reset(); sammccall wrote: > kadircet wrot

[PATCH] D123212: [clangd] Handle the new UsingTemplateName.

kadircet added a comment. thanks a lot for doing this! some drive-by comments Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:87 + // to handle the UsingTemplateName case. + bool TraverseTemplateName(TemplateName TN) { +if (const auto *UTN = TN.getAsUsingTemplateN

[PATCH] D130081: Add foldings for multi-line comment.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Protocol.h:1782 unsigned endCharacter; - std::string kind; + FoldingRangeKind kind; }; hokein wrote: > sorry for not being clear on my previous comment, I think the current `string > kind

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. Lots of featu

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1528 } - + auto MainFilePath = + getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM); dgoldman wrote: > dgoldman wrote: > > sammccall wrote: > > > kadircet wrote: >

[PATCH] D130636: [clangd] Upgrade vlog() to log() for preamble build stats

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Preamble.cpp:555 if (BuiltPreamble) { -vlog("Built preamble of size {0} for file {1} version {2} in {3} seconds", +log

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

kadircet marked 2 inline comments as done. kadircet added a comment. as discussed offline, this patch stores the filepath provided by the client (e.g. vscode) which is usually the filepath seen by the user, inside the ParsedAST and later on uses that when a hint is needed for translating filepa

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rG3b8fb471cbbd: [clangd][NFCI] Store TUPath inside ParsedAST (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

kadircet added a comment. Can you also add a test, preferably in clang/test/CodeCompletion/overrides.cpp ? Comment at: clang/lib/Parse/ParseDecl.cpp:3269 - if (getCurScope()->getFnParent() || getCurScope()->getBlockParent()) -CCC = Sema::PCC_LocalDeclarationSpeci

[PATCH] D130863: [clangd] Make git ignore index directories

kadircet added a comment. I am not sure if clangd is the right tool the create those `.gitignore` files, e.g. what if the project's VCS isn't git? I believe the right thing to do is for projects that want to make use of such tools to ignore the relevant directory explicitly (e.g. `.cache). Re

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

kadircet added a comment. sorry this has slipped through. since we're going to backport it, let's make it minimal and always point to in-progress release notes (so sorry for reverting back to original version of the patch). i think having clangd point to versioned docs would be much nicer, but

[PATCH] D130363: [clang] Give priority to Class context while parsing declarations

kadircet added a comment. > Is my invocation correct or is this case handled by clangd and not clang? yeah your invocation is correct. this is because clang's printer does a prefix based filtering (while clangd does a fuzzy match) https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/Co

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

kadircet added a comment. I agree with Sam on this one, especially on big files it just floods and hides non-stylistic diagnostics (you even hit max diagnostics exceeded errors). I believe unless someone takes the extra mile to fix all the value types in LLVM to be const-correct, this is harmfu

[PATCH] D130826: [clang-tools-extra] Fixed a number of typos

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/D130826/new/ https://reviews.llvm.org/D130826 __

[PATCH] D130826: [clang-tools-extra] Fixed a number of typos

kadircet added a comment. also LMK if I should land this for you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130826/new/ https://reviews.llvm.org/D130826 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D130826: [clang-tools-extra] Fixed a number of typos

This revision was automatically updated to reflect the committed changes. Closed by commit rG0ed2bd9311fd: [clang-tools-extra] Fixed a number of typos (authored by GabrielRavier, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

kadircet added a comment. no worries, as discussed offline this LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130041/new/ https://reviews.llvm.org/D130041 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

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

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

kadircet added a comment. Reverting for @bgraur as he's having some issues with his commit access ATM. To sum up the reproducer attached by @joanahalili triggers a crash https://reviews.llvm.org/rG4e94f6653150511de434fa7e29b684ae7f0e52b6 with stack trace and error: $ ~/repos/llvm/build/bin/c

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

kadircet added a comment. Reverted in df48e3fbcc8be1f4c04bd97517d12e662f54de75 , @tstellar it needs to be cherry-picked into release branch as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D131385: [clangd] Support for standard type hierarchy

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

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added a comment. thanks, i think we should change the behaviour to not fold the last line in any case. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:943 Callback> Reply) { - Server->foldingRanges(Params.textDocument.uri.file(), std::move(Reply)); + S

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:225 + Position End) { +// For LineFoldingsOnly clients, do not fold the last line if it +// contains tokens after `End`. hokei

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

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/CodeCompleteTests.cpp:2187 +TEST(CompletionTest, FixItForMembersUsing) { + const Annotations Code(R"cpp( --

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

kadircet added a comment. looks like you've uploaded the diff without context, can you upload it again with full context? also the changes to `MaybeAddResult` seem to be missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131088/new/ https://reviews.llvm.org/D131088 ___

[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

kadircet added a comment. I agree with Nathan to some extent that this doesn't carry as much weight, but if we can get it without introducing much complexity to the user (i.e. a new modifier they need to remember), it's worth having since we don't need too much effort to support it.

[PATCH] D131569: [clangd] Allow updates to be canceled after compile flags retrieval

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

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

kadircet added a comment. In D105904#2877281 , @dgoldman wrote: > Yep, I can you send you some searches internally with how prevalent they are. Thanks! > I don't think this an Xcode limitation really - more of an LSP spec > limitation. If we create a g

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

kadircet added a comment. > However those difficulties don't apply here - I believe you can take the > current final representation (vector) + the list of marks and > produce a markified vector via a fairly natural tree walk. > And this does a *really* good job of isolating this feature - we li

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

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! sorry for the late reply :( adding a couple nits. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:170 + while (Enclosing) { +const Named

[PATCH] D106227: Fix duplicate checks in clangd comments

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

[PATCH] D106201: [clangd] Add tests covering existing header-guard behavior. NFC

kadircet added a comment. i mostly agree with the desired behaviours laid out by the tests, mentioned a coupe extra cases and wrinkly looking parts in comments. Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:724 + + TU.Code = R"cpp( +#pragma once --

[PATCH] D106227: Fix duplicate checks in clangd comments

This revision was automatically updated to reflect the committed changes. Closed by commit rG195786d7c260: Fix duplicate checks in clangd comments (authored by 1ntEgr8, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106227/new/

[PATCH] D106201: [clangd] Add tests covering existing header-guard behavior. NFC

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:725 + TU.Code = R"cpp( +#pragma once +; sammccall wrote: > kadircet wr

[PATCH] D106203: [clangd] Propagate header-guarded flag from preamble to main AST

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/Preamble.cpp:87 + +const SourceManager &SM = CI.getSourceManager(); +const FileEntry *MainFE = SM.getFileEntryForID(S

[PATCH] D106527: [clangd] Canonicalize compile flags before applying 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. Pushes input for the compile ac

<    11   12   13   14   15   16   17   18   19   20   >