[PATCH] D104619: [clang] Respect PrintingPolicy::FullyQualifiedName when printing a template-id

nridge updated this revision to Diff 356604. nridge added a comment. Rebase on top of refactor patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104619/new/ https://reviews.llvm.org/D104619 Files: clang/lib/AST/TypePrinter.cpp clang/unittest

[PATCH] D104619: [clang] Respect PrintingPolicy::FullyQualifiedName when printing a template-id

nridge added a comment. In D104619#2846262 , @dblaikie wrote: > If there's already some duplication, perhaps a pre-patch to generalize that > functionality, then using that functionality in this patch? I posted the patch to generalize the AST printing t

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge updated this revision to Diff 356605. nridge added a comment. Move a piece of Decl-specific code into DeclPrinterTest.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 Files: clang/unittests/AS

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

nridge added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:489 DeclRefs[ND].push_back( -SymbolRef{SM.getFileLoc(Loc), Roles, ASTNode.Parent}); +SymbolRef{SM.getFileLoc(Loc), Roles, getRefContainer(ASTNode.Parent)}); // Don

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

nridge updated this revision to Diff 357823. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105083/new/ https://reviews.llvm.org/D105083 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp c

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

nridge marked an inline comment as done. nridge 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 *Enclos

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge updated this revision to Diff 357826. nridge marked 3 inline comments as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 Files: clang/unittes

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added inline comments. Comment at: clang/unittests/AST/DeclPrinterTest.cpp:59 + return PrintedNodeMatches(Code, Args, NodeMatch, ExpectedPrinted, + FileName, PrinterType{PrintDecl}, + PolicyModifier, AllowE

[PATCH] D104619: [clang] Respect PrintingPolicy::FullyQualifiedName when printing a template-id

nridge updated this revision to Diff 357827. nridge added a comment. Rebase on top of refactor patch changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104619/new/ https://reviews.llvm.org/D104619 Files: clang/lib/AST/TypePrinter.cpp clang/

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

nridge added a comment. Will building with this option just prevent linking the clang-tidy checks into clangd, or will it also prevent building them in the first place? In my experience, it's the building that's the primary compile time expense, so if the intention is to address https://github.

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20176bc7dd3f: [clang] Refactor AST printing tests to share more infrastructure (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. To be honest, I don't really understand this error. It seems to come from a compile step (not a link step), and it comes from the assembler and says "symbol ... is already defined"? I don't think I've seen an error like that before. (Typically, errors about duplicate def

[PATCH] D91186: [clangd] Add documentation for building and testing clangd

nridge added a comment. Herald added a project: clang-tools-extra. I wonder, should these instructions be in an easier-to-find place, such as the clangd repo's own README (https://github.com/clangd/clangd/blob/master/README.md), or even a section of the clangd website (clangd.llvm.org)? I want

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. Thanks! I reduced it further to: template class function { public: template function(F) {} }; template void Foo(M, function = [](){}); void Bar() { Foo(42); Foo(42.0); } with gcc 7, this gives: /tmp/cccKjL8O.s: Assembler message

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. Making the default argument a non-lambda seems to be sufficient to avoid the error: template class function { public: template function(F) {} }; void DefaultFunc(); template void Foo(M, function = DefaultFunc); void Bar() { Foo(42);

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge updated this revision to Diff 358750. nridge added a comment. Work around a gcc 7 bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 Files: clang/unittests/AST/ASTPrint.h clang/unittests/AST/D

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. In D105457#2878302 , @dyung wrote: > Interesting. If you can get me an updated patch, I can give it a try on my > machine with 7.5 to verify if you like. I posted an updated patch, if you can give that a whirl that would be great

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9cfec72ffeec: [clang] Refactor AST printing tests to share more infrastructure (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. @dyung I appreciate the help tracking this down! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 ___ cfe-commits mailing list cfe-commit

[PATCH] D111971: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing

nridge updated this revision to Diff 387057. nridge added a comment. Reworked test to use the framework in clang/test/AST Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111971/new/ https://reviews.llvm.org/D111971 Files: clang/lib/Serialization/A

[PATCH] D111971: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing

nridge requested review of this revision. nridge added a comment. Sorry for the delay here. @adamcz could you kindly have another look to make sure the test changes look ok? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111971/new/ https://review

[PATCH] D114058: [clang] Add ObjC decls to Decl::isFunctionOrFunctionTemplate

nridge added a comment. Thanks for the patch. Could you add a test exercising call hierarchy for obj-c methods to CallHierarchyTests.cpp please? In terms of the actual change, this function has some other callers

[PATCH] D111971: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a8c7946fc3a: [clang] Allocate 2 bits to store the constexpr specifier kind when serializing (authored by nridge). Repository: rG LLVM Github Mono

[PATCH] D114213: Compilation Database: Point Bazel users to a solution

nridge added a comment. In addition to these compilation database docs, I would also suggest adding mention of Bazel to https://clangd.llvm.org/installation#project-setup. (The repo for that it https://github.com/llvm/clangd-www/blob/main/installation.md and a change can be proposed using a Git

[PATCH] D114058: [clangd] Add ObjC method support to prepareCallHierarchy

nridge added a comment. Thanks, LGTM! I have a couple of nits about the test changes, but with those I think this is good to merged. Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:54 +void verifyIncomingMultiFile(std::string SourceExt, std::string Hea

[PATCH] D114058: [clangd] Add ObjC method support to prepareCallHierarchy

nridge added a comment. In D114058#3150606 , @kadircet wrote: > thanks, lgtm! let me know of your email address (for commit attribution) if > you want me to land this for you. While I don't see it surfaced anywhere in the Phabricator UI, there is in fac

[PATCH] D105177: [clangd] Implemented indexing of standard library

nridge added a comment. What is the status of this -- is it ready to be merged? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105177/new/ https://reviews.llvm.org/D105177 ___ cfe-commits mailing list cfe

[PATCH] D114213: Compilation Database: Point Bazel users to a solution

This revision was automatically updated to reflect the committed changes. Closed by commit rG5233ad17e77e: Compilation Database: Point Bazel users to a solution (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114213/new/ https://

[PATCH] D114213: Compilation Database: Point Bazel users to a solution

nridge added a comment. In D114213#3156940 , @cpsauer wrote: > could I ask for your help landing the change now that it's approved? Landed! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114213/new/ https:

[PATCH] D114667: [clangd] Add fixes for clang "include " diagnostics

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks, this is nice low-hanging fruit. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:830 + if (!ReplacementFixes.empty()) { +LastDiag->Fixes.insert(LastD

[PATCH] D136212: [clangd] consider ~^foo() to target the destructor, not the type

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks for fixing! Comment at: clang-tools-extra/clangd/Selection.cpp:881 + return CDD->getNameInfo().getNamedTypeInfo()->getTypeLoc().getBeginLoc(); +if (const Stmt

[PATCH] D133756: [clangd] Introduce CompileCommandsAdjuster

nridge updated this revision to Diff 470083. nridge marked 4 inline comments as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133756/new/ https://reviews.llvm.org/D133756 Files: clang-tools-e

[PATCH] D133756: [clangd] Introduce CompileCommandsAdjuster

nridge added a comment. (apologies, I'm also very tardy in getting back to this...) Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:165 +// process a file (possibly different from the one in the command). +class CompileCommandsAdjuster { +public: --

[PATCH] D133756: [clangd] Pass the entire tooling::CompileCommand to CommandMangler

nridge added a comment. (I frequently forget that `arc diff` does not update the commit message of an existing revision...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133756/new/ https://reviews.llvm.org/D133756 ___

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

nridge added a comment. For reference, earlier work along these lines which never landed: https://reviews.llvm.org/D119077 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136594/new/ https://reviews.llvm.org/D136594

[PATCH] D136925: [clangd] Index scoped enums for code completion

nridge added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:2133 if (const auto *EnumDecl = dyn_cast(ND.getDeclContext())) -return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped(); Just to make sure I understand: By also remov

[PATCH] D134827: [clangd] Avoid recursion on UnresolvedUsingValueDecl during semantic highlighting

nridge requested review of this revision. nridge added a comment. (I'm going to re-request review since I have an outstanding question about the fix approach.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134827/new/ https://reviews.llvm.org/D134

[PATCH] D136925: [clangd] Index scoped enums for code completion

nridge added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:2133 if (const auto *EnumDecl = dyn_cast(ND.getDeclContext())) -return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped(); tom-anders wrote: > nridge wrote: > > Just to

[PATCH] D137056: [clangd] Fix a small inconsistency in system-include-extractor.test

nridge created this revision. nridge added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Also add an explana

[PATCH] D137056: [clangd] Fix a small inconsistency in system-include-extractor.test

nridge updated this revision to Diff 471908. nridge added a comment. format comment better Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137056/new/ https://reviews.llvm.org/D137056 Files: clang-tools-extra/clangd/test/system-include-extractor.t

[PATCH] D137056: [clangd] Fix a small inconsistency in system-include-extractor.test

nridge updated this revision to Diff 471909. nridge added a comment. fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137056/new/ https://reviews.llvm.org/D137056 Files: clang-tools-extra/clangd/test/system-include-extractor.test Index:

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

nridge updated this revision to Diff 471925. nridge added a comment. reformulate test as lit test address other review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133757/new/ https://reviews.llvm.org/D133757 Files: clang-tools-extra/c

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

nridge added a comment. I believe this addresses the remaining review comments. I will follow up with a patch to rename QueryDriverDatabase.{h,cpp} to SystemIncludeExtractor,{h,cpp}. Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:318 /// compilation database. -c

[PATCH] D133756: [clangd] Pass the entire tooling::CompileCommand to CommandMangler

nridge added inline comments. Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:165 +// process a file (possibly different from the one in the command). +class CompileCommandsAdjuster { +public: sammccall wrote: > nridge wrote: > > sammccall wrote:

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

nridge added inline comments. Comment at: clang-tools-extra/clangd/test/system-include-extractor.test:82 + +# Skip past the lack of diagnostics in the workspace and user config files... +# CHECK2: "method": "textDocument/publishDiagnostics", Ugh, this doesn't q

[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

nridge added a comment. In D121593#3896892 , @Abpostelnicu wrote: > was there any progress on this so far? This would be really useful to be a > clang-tidy check. My impression is that it's actively being worked on: https://reviews.llvm.org/search/que

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

nridge updated this revision to Diff 472235. nridge added a comment. Improve handling of config file diagnostics in lit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133757/new/ https://reviews.llvm.org/D133757 Files: clang-tools-extra/clan

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

nridge added inline comments. Comment at: clang-tools-extra/clangd/test/system-include-extractor.test:82 + +# Skip past the lack of diagnostics in the workspace and user config files... +# CHECK2: "method": "textDocument/publishDiagnostics", nridge wrote: > Ugh

[PATCH] D136925: [clangd] Index unscoped enums inside classes for code completion

nridge added inline comments. Comment at: clang-tools-extra/clangd/index/Serialization.cpp:458 // data. Later we may want to support some backward compatibility. -constexpr static uint32_t Version = 17; +constexpr static uint32_t Version = 18; I don't think in

[PATCH] D136925: [clangd] Index unscoped enums inside classes for code completion

nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2966 {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"), - cls("na::nb::Clangd4")}, + cls("na::nb::Clangd4"), enmConstant("na::C::Clangd5")}, Opts); -

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

nridge added a comment. Related issue: https://github.com/clangd/clangd/issues/826 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131385/new/ https://reviews.llvm.org/D131385 ___ cfe-commits mailing list

[PATCH] D133756: [clangd] Introduce CompileCommandsAdjuster

nridge added inline comments. Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:165 +// process a file (possibly different from the one in the command). +class CompileCommandsAdjuster { +public: sammccall wrote: > I have a couple of concerns with t

[PATCH] D131853: [clangd] Add doxygen parsing for Hover

nridge added a comment. In D131853#3792985 , @logankaser wrote: > Is there anything I can do as a random member of the public that wants this > and knows C++? Maybe apply the patch locally, use it for a bit, and provide feedback? I haven't forgotten a

[PATCH] D133982: [clangd] Improve inlay hints of things expanded from macros

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks for fixing! Comment at: clang-tools-extra/clangd/InlayHints.cpp:281 - void addReturnTypeHint(FunctionDecl *D, SourceLocation Loc) { + void addReturnTypeHint(Functi

[PATCH] D134137: [clangd] Return earlier when snippet is empty

nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1017 +TEST(CompletionTests, EmptySnippetDoesNotCrash) { +// See https://github.com/clangd/clangd/issues/1216 Does this test trigger the crash for you when run

[PATCH] D134137: [clangd] Return earlier when snippet is empty

nridge accepted this revision. nridge added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1017 +TEST(CompletionTests, EmptySnippetDoesNotCrash) { +// See https://github.com/clangd/clang

[PATCH] D133756: [clangd] Introduce CompileCommandsAdjuster

nridge added inline comments. Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:165 +// process a file (possibly different from the one in the command). +class CompileCommandsAdjuster { +public: nridge wrote: > sammccall wrote: > > I have a couple

[PATCH] D133756: [clangd] Introduce CompileCommandsAdjuster

nridge updated this revision to Diff 462702. nridge added a comment. Updated to use unique_function rather than inheritance Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133756/new/ https://reviews.llvm.org/D133756 Files: clang-tools-extra/clang

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

nridge updated this revision to Diff 462704. nridge marked 2 inline comments as done. nridge added a comment. This is mostly just a rebase on top of the update to D133756 to turn CompileCommandsAdjuster into a unique_function, and addressing a few minor comments

[PATCH] D134827: [clangd] Avoid recursion on UnresolvedUsingValueDecl during semantic highlighting

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

[PATCH] D134827: [clangd] Avoid recursion on UnresolvedUsingValueDecl during semantic highlighting

nridge added a comment. Speculative fix for an infinite recursion with no testcase. I'm happy to circle back to this if/when we get a testcase, but I don't think there's any downside to this patch in the meantime. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D134827: [clangd] Avoid recursion on UnresolvedUsingValueDecl during semantic highlighting

nridge added a comment. Thanks for the testcases! The indirect one is particularly interesting, and suggests that we may want a "seen decls" set (similar to `SeenTemplates` here ) in t

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

This revision was automatically updated to reflect the committed changes. Closed by commit rG4abc910a42e5: [clangd] Implement semantic token modifier "definition" (authored by ckandeler, committed by nridge). Changed prior to commit: https://reviews.llvm.org/D127403?vs=466731&id=468318#toc Rep

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. I went ahead and landed this for you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. Looks like an error introduced when rebasing the patch: a recently added test case (for https://github.com/clangd/clangd/issues/1222) needs to be updated to reflect the `_decl` to `_def` change that this patch makes to all semantic highlighting tests. I'll land a fix.

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. Landing the fix now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. Landed https://reviews.llvm.org/rGc93430bae4fc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 ___ cfe-commits mailing list cfe-commits@

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

nridge added a comment. In D127403#3863686 , @thakis wrote: > In D127403#3863641 , @nridge wrote: > >> Landed https://reviews.llvm.org/rGc93430bae4fc > > Still failing with that: http://45.33.8.238/linux/89240/ste

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)`

nridge accepted this revision. nridge 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/D138300/new/ https://reviews.llvm.org/D138300

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)`

This revision was automatically updated to reflect the committed changes. Closed by commit rG4b2cf982cc51: [clangd] Support type hints for `decltype(expr)` (authored by v1nh1shungry, committed by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)`

nridge added a comment. Done, thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138300/new/ https://reviews.llvm.org/D138300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D140379: [clangd] Avoid triggering linkage computation for decl with unstable linkage in SymbolRelevanceSignals::computeASTSignals()

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

[PATCH] D140379: [clangd] Avoid triggering linkage computation for decl with unstable linkage in SymbolRelevanceSignals::computeASTSignals()

nridge added inline comments. Comment at: clang-tools-extra/clangd/Quality.cpp:306 if (const NamedDecl *ND = SemaResult.getDeclaration()) { +if (hasUnstableLinkage(ND)) + return; Would it be better to reuse [this getSymbolID overload](https://searchf

[PATCH] D140379: [clangd] Avoid triggering linkage computation for decl with unstable linkage in SymbolRelevanceSignals::computeASTSignals()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6761ba7639a4: [clangd] Avoid triggering linkage computation for decl with unstable linkage in… (authored by nridge). Repository: rG LLVM Github Mo

[PATCH] D140462: [clangd] Add schema for `.clangd` config

nridge added a subscriber: sammccall. nridge added a comment. Thanks, this is pretty neat! cc @sammccall as a heads up, just to make sure you're cool with having this in-tree Comment at: clang-tools-extra/clangd/schema/config.json:48 + "description": "Directory to se

[PATCH] D140462: [clangd] Add schema for `.clangd` config

nridge added a comment. In D140462#4012996 , @sammccall wrote: > This has been discussed before, unfortunately while I *thought* it was on a > bug, it was actually in a PR: https://github.com/clangd/vscode-clangd/pull/140 > > I don't think it's a good id

[PATCH] D140462: [clangd] Add schema for `.clangd` config

nridge added a comment. In D140462#4014944 , @sammccall wrote: > given lack of any automation/tests That's a fair point, it would definitely help validate the correctness of the schema if we had tests in the form of example config files that pass or fai

[PATCH] D140619: ExtractFunction: support extracting expressions and selected part of it

nridge added a comment. Linking to potentially relevant issue on file: https://github.com/clangd/clangd/issues/1254 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140619/new/ https://reviews.llvm.org/D140619 ___

[PATCH] D140462: [clangd] Add schema for `.clangd` config

nridge added a comment. In D140462#4017237 , @hyperupcall wrote: > If you would like me to add tests to verify the schema (for now or later?), > is there a utility within LLVM to help do so? I saw TableGen was mentioned, > but it sounds like the schema

[PATCH] D140462: [clangd] Add schema for `.clangd` config

nridge added a comment. In D140462#4017240 , @nridge wrote: > Is there a command-line tool that can perform JSON schema validation (well, more specifically that can validate YAML files against a JSON schema) Repository: rG LLVM Github Monorepo CHANG

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

nridge added a comment. In D138546#4017356 , @ArcsinX wrote: > In other words, with this patch we preserve `--target=...`, but this option > can appear from > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/GlobalCompilationDatab

[PATCH] D140462: [clangd] Add schema for `.clangd` config

nridge added a comment. In D140462#4018983 , @sammccall wrote: > I tried 3 consumers (`ajv-cli`, `yaml-language-server`, and `yajsv`) and > **hit different blocking bugs in all of them** when using obvious, > spec-compliant approaches to writing schemas

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. LGTM, and it looks like Sam's comments have been substantially addressed as well, so I'm going to go ahead and approve this, hope that's cool. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

nridge added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:779 -HoverInfo getStringLiteralContents(const StringLiteral *SL, - const PrintingPolicy &PP) { - HoverInfo HI; - +void addStringLiteralContents(const StringLiteral *S

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

nridge added a comment. Do you want to add a simple test case for a non-literal expression? Something like hovering over the `+` in `2 + 3` should work. Also, this is pre-existing, but I noticed that in a case like this: void bar(int); void func() { int x = 42; bar(x); } the hove

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

nridge added a comment. Thanks, I think this behaviour change is reasonable. Comment at: clang-tools-extra/clangd/InlayHints.cpp:667 +// print the underlying type for `decltype(expr)` +if (T->isDecltypeType()) + TypeHintPolicy.PrintCanonicalTypes = true; --

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

nridge added a comment. In D140775#4021967 , @tom-anders wrote: > In D140775#4021634 , @nridge wrote: > >> Do you want to add a simple test case for a non-literal expression? >> Something like hovering over the

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang-tools-extra/clangd/InlayHints.cpp:665 + // The sugared type is more useful in some cases, and the canonical + // type in other cases. For now, p

[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

This revision was automatically updated to reflect the committed changes. Closed by commit rGdde8a0fe91cc: [clangd] show underlying type in type hint for `decltype(expr)` (authored by v1nh1shungry, committed by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

nridge accepted this revision. nridge 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/D140775/new/ https://reviews.llvm.org/D140775

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

nridge added a comment. I'm happy to review the implementation, but I would first appreciate some guidance from @sammccall or @kadircet about whether we should add these semantic token kinds in the first place. They would be a non-standard extension to the LSP token kinds, so I think the use c

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

nridge added a comment. In D139926#4032473 , @ckandeler wrote: > In D139926#4030782 , @nridge wrote: > >> It's true that there is an ambiguity between `<` and `>` as operators, vs. >> template arg/param list deli

[PATCH] D141218: [clangd] Include the correct header for typeid()

nridge created this revision. nridge added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Fixes https://githu

[PATCH] D141218: [clangd] Include the correct header for typeid()

nridge updated this revision to Diff 487571. nridge added a comment. Remove test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141218/new/ https://reviews.llvm.org/D141218 Files: clang-tools-extra/clangd/IncludeFixer.cpp Index: clang-tools-ext

[PATCH] D141218: [clangd] Include the correct header for typeid()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG056d4dca7764: [clangd] Include the correct header for typeid() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D141495: [clangd] Suppress clang-tidy warnings for code spelled in system macros

nridge added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:545 } + // Match behavior for clang-tidy --system-headers=0 (the default). + if (Info.hasSourceManager() && Why not check `CTContext->getOptions().System

[PATCH] D136925: [clangd] Index unscoped enums inside classes for code completion

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! I don't think we need this much detail in the commit message; I think after the first line it would be sufficient to say: Fixes https://github.com/clangd/clangd/issues/1082 See th

[PATCH] D137104: [clangd] Add scoped enum constants to all-scopes-completion

nridge added a comment. The test added in the previous patch, `CompletionTest.Enums`, needs to be updated to reflect this change (`Scoped::Clangd3` now appears as a completion). Comment at: clang-tools-extra/clangd/CodeComplete.cpp:2136 + if (llvm::isa(ND.getDeclContext())) +

[PATCH] D137104: [clangd] Add scoped enum constants to all-scopes-completion

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Looks good, thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137104/new/ https://reviews.llvm.org/D137104 _

[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

nridge added a comment. Thanks for the patch! The test looks good to me. As for the fix, I wonder if it would make sense to implement it in the Sema layer rather than in clangd. For example, we could give `clang::CodeCompletionResult` a new flag

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