[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

nridge updated this revision to Diff 551377. nridge added a comment. Rebase and address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147905/new/ https://reviews.llvm.org/D147905 Files: clang-tools-extra/clangd/SystemIncludeExtrac

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbd74186f1a08: [clangd] Avoid passing -xobjective-c++-header to the system include extractor (authored by nridge). Repository: rG LLVM Github Monor

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

nridge created this revision. nridge added reviewers: sammccall, hokein. 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. Re

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:466 +// so that we can recover argument names from it. +// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify. +static FunctionProtoTypeLoc getPrototypeLoc(Expr *Callee) {

[PATCH] D158248: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr()

nridge created this revision. nridge added reviewers: sammccall, hokein. 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. Pl

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:466 +// so that we can recover argument names from it. +// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify. +static FunctionProtoTypeLoc getPrototypeLoc(Expr *Callee) {

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

nridge updated this revision to Diff 551390. nridge added a comment. Fix naming nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158249/new/ https://reviews.llvm.org/D158249 Files: clang-tools-extra/clangd/InlayHints.cpp clang-tools-extra/cla

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

nridge requested changes to this revision. nridge added a comment. This revision now requires changes to proceed. Ok, I've finished looking through the patch; sorry it took so long to get to. It generally looks pretty good to me, I just have some fairly minor comments. Thanks again for your work

[PATCH] D158248: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr()

This revision was automatically updated to reflect the committed changes. Closed by commit rG3e69886dd012: [clangd] Fix incorrect RecursiveASTVisitor usage in summarizeExpr() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158248/

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

nridge updated this revision to Diff 551694. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149236/new/ https://reviews.llvm.org/D149236 Files: clang-tools-extra/clangd/ParsedAST.cpp clang-tools-ex

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG744b111434b2: [clangd] Bail gracefully if given an assembly or IR source file (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

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

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:508 + // Only one of Callee or ProtoTypeLoc is set. + const FunctionDecl *Callee = nullptr; + FunctionProtoTypeLoc ProtoTypeLoc; sammccal

[PATCH] D158249: [clangd] Parameter hints for calls through function pointers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. nridge marked an inline comment as done. Closed by commit rG8ee710a40cc5: [clangd] Parameter hints for calls through function pointers (authored by nridge). Repository

[PATCH] D157956: [clangd] don't add inlay hint for dependent type in structured binding

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! A future enhancement to consider could be, in the following testcase: template struct Pair { T t; U u; }; template void foobar(Pair arg) { auto [a, b] = arg;

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks for the patch! My thoughts are: - As mentioned in the issue, I think this fills a logical gap: clangd-indexer is an alternative way of generating a project index to clangd's background

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

nridge added a comment. In D153114#4603579 , @sammccall wrote: > Dep scanning - roles > > > IIUC we do this for two reasons: > > - to identify what module names we must have PCMs for in order to build a > given TU (either an open fil

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge commandeered this revision. nridge added a reviewer: qchateau. nridge added a comment. In D93829#4422090 , @qchateau wrote: > So if anyone wants to take it from there, feel free to reuse my commits (or > not). I'm going to pick up this work and tr

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge added a comment. To be able to reason about the impact of various memory usage optimizations, I took some baseline measurements. I used a workload consisting of the LLVM codebase, with the compile_commands.json entries filtered to those containing `clang/lib` or `clang-tools-extra/clang

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge updated this revision to Diff 557745. nridge added a comment. Implement optimization to filter the refs stored in RevRefs to calls only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files: clang-

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge planned changes to this revision. nridge added a comment. The updated patch implements one of the optimizations discussed during review, namely filtering the Refs stored in the RevRefs data structure to just those that could be calls. To this end, the patch introduces a new `RefKind`, `R

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

nridge added a comment. In D150124#4329163 , @ilya-biryukov wrote: > In `clangd` we also have `findExplicitReferences` and `targetDecl` functions > that seem to handle the `GoToStmt`. > In fact, I believe they are used in `findDocumentHighlights`, so I'

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

nridge added a reviewer: sammccall. nridge added a comment. Adding Sam as well in case he has any thoughts on the discussion (another user ran into this recently and filed https://github.com/clangd/clangd/issues/1694) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D156513: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext::handleReference()

nridge created this revision. nridge added a reviewer: hokein. Herald added a subscriber: arphaman. Herald added a project: All. nridge requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.l

[PATCH] D156513: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext::handleReference()

nridge added a comment. Noticed this while reviewing D150124 . No caller was providing a value for this parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156513/new/ https://reviews.llvm.org/D156513 __

[PATCH] D156513: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext::handleReference()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5ddc37189032: [clang] [libIndex] Remove unused 'RefD' parameter of IndexingContext… (authored by nridge). Repository: rG LLVM Github Monorepo CHA

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

nridge added inline comments. Comment at: clang/lib/Index/IndexBody.cpp:150 +ParentDC, +unsigned(SymbolRole::NameReference)); + } ckandeler wrote: > kadircet wrote: > > nridge wrote: > > > `

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

nridge added a comment. In D150124#4542032 , @kadircet wrote: > btw, thanks a lot Nathan for taking good care of clangd, I don't think I say > that enough I'm happy to help! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D156300: [clangd] Avoid unexpected desugaring in isSugaredTemplateParameter

nridge added a comment. Your patience is appreciated. I have a number of patches in my review queue, and 3 days is often not a realistic turnaround time for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156300/new/ https://reviews.llvm.org/D15

[PATCH] D155370: [CodeComplete] Improve FunctionCanBeCall

nridge added a comment. I'm not sure how I feel about dropping the parameters from the signature in the `CanBeCall = false` case. I can see arguments in both directions: - On the one hand, dropping the parameters makes the text that is inserted more consistent with the text that is shown for t

[PATCH] D156300: [clangd] Avoid unexpected desugaring in isSugaredTemplateParameter

nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:198 bool isSugaredTemplateParameter(QualType QT) { static auto PeelWrappers = [](QualType QT) { // Neither `PointerType` nor `ReferenceType` is considered as sugared nit: s

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150124/new/ https://reviews.llvm.org/D150124 ___

[PATCH] D156300: [clangd] Avoid unexpected desugaring in isSugaredTemplateParameter

nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1432 TEST(TypeHints, SubstTemplateParameterAliases) { + llvm::StringRef Header = R"cpp(

[PATCH] D155370: [CodeComplete] Improve FunctionCanBeCall

nridge added a comment. In D155370#4550042 , @zyounan wrote: > in the context where `CanBeCall=false`, parameters don't disambiguate against > the overloads, so we'd end up with the same function name after selecting the > candidate I was thinking of f

[PATCH] D139277: [clangd] Use all query-driver arguments in cache key

nridge added a comment. This was superseded by D146941 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139277/new/ https://reviews.llvm.org/D139277 ___ cfe-commits maili

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

nridge accepted this revision. nridge added a comment. I'm fine with allowing partial selection for consistency with other expressions. I think this patch is in a good state. Thanks for your patience with the review :) Let me know if you need me to commit it for you. Repository: rG LLVM Gith

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG94b14355e2ef: [clangd] allow extracting to variable for lambda exp

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

nridge added a comment. This can be closed, as the change has been made in https://github.com/llvm/llvm-project/pull/65824 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138546/new/ https://reviews.llvm.org/D138546 ___ cfe-commits mailing lis

[PATCH] D149236: [clangd] Bail gracefully if given an assembly or IR source file

nridge added a comment. Review ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149236/new/ https://reviews.llvm.org/D149236 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D154471: [clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

nridge created this revision. Herald added a project: All. nridge updated this revision to Diff 537970. nridge added a comment. nridge edited the summary of this revision. Herald added a subscriber: kadircet. nridge published this revision for review. Herald added subscribers: cfe-commits, ilya-bir

[PATCH] D154903: clangd: Provide the resource dir via environment variable

nridge added a reviewer: sammccall. nridge added a comment. Looks reasonable to me but Sam should OK this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154903/new/ https://reviews.llvm.org/D154903 ___ c

[PATCH] D154471: [clang] Add serialization support for the DynamicAllocLValue variant of APValue::LValueBase::Ptr

nridge planned changes to this revision. nridge added a comment. Thank you for the suggested testcase! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154471/new/ https://reviews.llvm.org/D154471 ___ cfe-c

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

nridge resigned from this revision. nridge added a comment. I'm sorry, I feel like I don't have a good enough level of insight into the requirements to usefully critique this patch, nor the bandwidth to take a detailed look through the implementation right now. I think it's best for me to resig

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

nridge added reviewers: hokein, kadircet, VitaNuo. nridge added a comment. Let's try adding some reviewers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78038/new/ https://reviews.llvm.org/D78038 ___ cfe

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

nridge added a comment. Also adding links to the open issues this fixes: - https://github.com/clangd/clangd/issues/333 - https://github.com/clangd/clangd/issues/337 Note the second issue is not specific to include insertion; it's a false positive diagnostic, which I think makes it a more severe

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:581 + +[[Fooable]] auto i = 42; + )cpp"; sammccall wrote: > this is going to have the same behavior on the `auto` token, right? > > This is my main practical co

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:581 + +[[Fooable]] auto i = 42; + )cpp"; nridge wrote: > sammccall wrote: > > this is going to have the same behavior on the `auto` token, right? > > > > This i

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

This revision was automatically updated to reflect the committed changes. Closed by commit rG109bc024c8d7: [clangd] Add --query-driver flag to clangd-indexer (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157990/new/ https://rev

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

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

[PATCH] D158851: [clangd] Avoid null result in FindRecordTypeAt()

nridge created this revision. nridge added reviewers: kadircet, hokein. 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. Repository:

[PATCH] D158851: [clangd] Avoid null result in FindRecordTypeAt()

This revision was automatically updated to reflect the committed changes. Closed by commit rG1994e1173b64: [clangd] Avoid null result in FindRecordTypeAt() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158851/new/ https://revie

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

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. https://g

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

nridge updated this revision to Diff 553607. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158873/new/ https://reviews.llvm.org/D158873 Files: clang-tools-extra/clangd/HeuristicResolver.cpp clang-

[PATCH] D158873: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf0f53cb4bfb2: [clangd] Do not ignore qualifier in heuristic resolution of dependent MemberExpr (authored by nridge). Repository: rG LLVM Github Mo

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

nridge added a comment. (I'm away on travels, will get back to this within the next week) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156605/new/ https://reviews.llvm.org/D156605 ___ cfe-commits mailin

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

nridge added a comment. In D153114#4630318 , @ChuanqiXu wrote: > However, I can't search the caller of `reparseOpenFilesIfNeeded` which > semantics matches the behavior. The two callers of > `reparseOpenFilesIfNeeded` I found are > `ClangdLSPServer::a

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

nridge added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:102 + + if (clang::Expr *const RequiresClause = + LExpr->getTrailingRequiresClause(); nit: just `if (clang::Expr *const RequiresClause = LExp

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

nridge requested changes to this revision. nridge added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:187 +// Allow expressions, but only allow completely selected lambda +/

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

nridge added a comment. (Otherwise the updates look good!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D141757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

nridge requested changes to this revision. nridge added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1297 +FunctionCanBeCall = +MaybeDerived == MaybeBase || MaybeDerived->isDerivedFrom(MaybeBase); + }

[PATCH] D158926: [clangd] Show parameter hints for operator()

nridge accepted this revision. nridge added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/InlayHints.cpp:626 + Method && Method->isInstance()) +Args = Args.drop_front(1); +processCall(Callee, Args); ---

[PATCH] D158804: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token is not an annotation token

nridge accepted this revision. nridge added a subscriber: rjmccall. nridge added a comment. This revision is now accepted and ready to land. I'm not well versed in the parser code, but the fix clearly avoids the assertion in `getIdentifierInfo()`, and both @aaron.ballman and @rjmccall indicated

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

nridge added a comment. A couple of high-level thoughts on this: 1. Based on the discussion in https://github.com/clangd/clangd/issues/1115, I believe highlighting of **built-in** operators should be out of scope for semantic highlighting, at least in the default mode; client-side highlighting

[PATCH] D138425: [clangd] Parameter hints for template specialization

nridge added a comment. FWIW, in codebases/components where I do code reviews, I tend to insist on template parameters having meaningful (and longer-than-one-character) names, and would find this quite useful. I agree that while template parameters are technically a kind of parameter, they're

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

nridge added a comment. In D136594#3940482 , @sammccall wrote: > I think since LSP specifies an `operator` SemanticTokenType, we should use it > unless there's a *really* strong reason not to. My bad, I completely overlooked that LSP has a standard toke

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

nridge added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76 ConstructorOrDestructor, + UserProvided, ckandeler wrote: > ckandeler wrote: > > sammccall wrote: > > > sammccall wrote: > > > > sammccall wrote: > > > > > Can you

[PATCH] D137770: [docs] Introduce clangd part in StandardCPlusPlusModules

nridge added a comment. In D137770#3945095 , @spartacoos wrote: > I realised this is an active issue that's not being worked on. So, I want to > put myself forward to help implement this new module system for clangd once > you have settled on a design/

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

nridge added a comment. In D138546#3946144 , @sammccall wrote: > In D138546#3946046 , @cpsauer wrote: > >> Sam, my read, too, is that the memoizing design isn't safe--also that the >> key bug is preexisting. >>

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

nridge added a comment. In D136594#3940482 , @sammccall wrote: > An (IMO) useful distinction that can't be found by the lexer is the use of > `*` as a declarator (`int *x`) vs an operator (`return *x`). I failed to appreciate the implication of this the

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

nridge added a comment. (I envisioned the implementation of `augmentsSyntaxTokens=false` to be "loop over the lexer tokens and turn a subset of them into additional tokens to return over LSP". The fact that that would have meant producing an "operator" token even for `tok::star` which is actual

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge updated this revision to Diff 253005. nridge added a comment. Finish an incomplete variable extraction Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75286/new/ https://reviews.llvm.org/D75286 Files: clang-tools-extra/clang-tidy/ClangTidyD

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge requested review of this revision. nridge added a comment. Thanks for the suggested simplification. As the change is not completely trivial, could you have one more look before landing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75286/new

[PATCH] D76896: Color dependent names based on their heuristic target if they have one

nridge created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/297 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76896 Files: clang-too

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710 +return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, +M1.Message) < + std::tie(M2.

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710 +return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, +M1.Message) < + std::tie(M2.

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710 +return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, +M1.Message) < + std::tie(M2.

[PATCH] D77023: clang-format fixes in ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp

nridge created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D77023 Files: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-ex

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge updated this revision to Diff 253446. nridge added a comment. Address last review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75286/new/ https://reviews.llvm.org/D75286 Files: clang-tools-extra/clang-tidy/ClangTidyDiagnosticCon

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

This revision was automatically updated to reflect the committed changes. Closed by commit rGb9d9968f63ab: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro… (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D77023: clang-format fixes in ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp

This revision was automatically updated to reflect the committed changes. Closed by commit rG15f1fe1506f5: clang-format fixes in ClangTidyDiagnosticConsumer.cpp and DiagnosticsTets.cpp (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D77194: [clang] Persist Attr::IsPackExpansion into the serialized AST

nridge created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/309 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D77194 Files: clang/utils/TableGen/ClangAtt

[PATCH] D77194: [clang] Persist Attr::IsPackExpansion into the serialized AST

nridge updated this revision to Diff 254712. nridge added a comment. Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77194/new/ https://reviews.llvm.org/D77194 Files: clang/test/PCH/cxx-attrs-packexpansion.cpp clang/utils/TableGen/Clang

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

nridge added a comment. I'm not sure that I'm qualified to approve this patch (this is my first time looking at clang's completion code), but I did look through the entire patch now, and it looks good to me modulo the mentioned, mostly minor, comments. Comment at: clang/lib/S

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Not handling this was a side-effect of being overly cautious when trying to avoid reading files f

[PATCH] D75292: [clangd] Remove the word 'toy' from the description of vscode-clangd

nridge created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. nridge added a comment. We could go a step further and describe it as the "reference client" for clangd, but I'm not sure how accura

[PATCH] D75292: [clangd] Remove the word 'toy' from the description of vscode-clangd

nridge added a comment. We could go a step further and describe it as the "reference client" for clangd, but I'm not sure how accurate / desirable that is at this stage. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75292/new/ https://reviews.llvm

[PATCH] D67537: [clangd] Client-side support for inactive regions

nridge updated this revision to Diff 247077. nridge added a comment. Update to use 'clangd.preprocessor.inactive' as the scope name for inactive lines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67537/new/ https://reviews.llvm.org/D67537 Files:

[PATCH] D67537: [clangd] Client-side support for inactive regions

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:193 + if (scopes[0] == "meta.disabled") { +this.inactiveDecorationIndex = index; +return vscode.win

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

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

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge marked 4 inline comments as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:261 + // consider it selected. + if (!SeenMacroCalls.insert(ArgStart).second) { +return NoTokens; sammccall wrote: > sammcc

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

nridge updated this revision to Diff 247529. nridge added a comment. Address some review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72874/new/ https://reviews.llvm.org/D72874 Files: clang-tools-extra/clangd/SourceCode.cpp clang-too

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

nridge added a comment. I've addressed some of the review comments, with a view to getting something minimal we can land, and improve on in follow-up changes. Mostly, I focused on the suggestions which reduce the number of results. I've left other suggestions which increase the number of result

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:287 if (SM.getFileID(ArgStart) == SelFile) { - SourceLocation ArgEnd = SM.getTopMacroCallerLoc(Batch.back().location()); - return testTokenRang

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

nridge added a comment. Thanks for all the comments Sam! I'll have a detailed look tomorrow, but I wanted to follow up on this: In D72874#1901383 , @sammccall wrote: > I think having this trigger where the identifier is an actual token in the > program

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

nridge marked 3 inline comments as done. nridge added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4856 +private: + // Infer members of T, given that the expression E (dependent on T) is true. + void believe(const Expr *E, const TemplateTypeParmType *T) { --

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

nridge added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4972 + +// In T::foo::bar, `foo` must be a type. +bool VisitNestedNameSpecifier(NestedNameSpecifier *NNS) { nridge wrote: > sammccall wrote: > > nridge wrote: > > > It would be

[PATCH] D75292: [clangd] Remove vsc-extension-quickstart.md from the vscode-clangd plugin

nridge updated this revision to Diff 247996. nridge added a comment. Remove entire file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75292/new/ https://reviews.llvm.org/D75292 Files: clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

nridge added a comment. Thanks! Consider the patch "accepted" from my POV. One last nit: please link to https://github.com/clangd/clangd/issues/261 in the commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73649/new/ https://reviews.llv

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

This revision was automatically updated to reflect the committed changes. nridge marked 4 inline comments as done. Closed by commit rGe70a9f385025: [clangd] Handle go-to-definition in macro invocations where the target appears… (authored by nridge). Changed prior to commit: https://reviews.llvm

<    5   6   7   8   9   10   11   12   13   14   >