[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

2023-04-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D148489#4276634 , @kadircet wrote: > before you dive any deeper into the patch, could you give some reasoning > about why this is needed/useful? https://github.com/clangd/clangd/discussions/1598 contains some context/motivati

[PATCH] D148284: [clangd] Add "readonly" token to const member expressions

2023-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks for the patch! `addExtraModifier` is a kind of hacky mechanism to attach modifiers in `CollectExtraHighlightings` to tokens created during `findExplicitReferences()`; it would be nicer to add the modifier at the time of creating the token, but it doesn't look lik

[PATCH] D148457: [clangd] Support macro evaluation on hover

2023-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks for the patch and the thorough testcase! I wonder if we should make this a bit more strict: if the macro expansion itself is an expression, show the value of //that// expression, but not e.g. an enclosing expression. Otherwise, the `Definition` field of the hover

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

2023-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: hokein, 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. The previous

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

2023-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:859 + // The rest is a simplified version of TestTU::build(). + // Don't call TestTU::build() itself because it would assert on + // failure to build an AST. (I cou

[PATCH] D147847: [clangd] Hover: Add CalleeArgInfo for constructor expressions

2023-04-28 Thread Nathan Ridge via Phabricator via cfe-commits
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/D147847/new/ https://reviews.llvm.org/D147847 ___

[PATCH] D151785: [clangd] Desugar dependent type aliases for auto type hints

2023-06-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks, this is a nice improvement. A couple of general comments: - We have an existing heuristic here , should we move the logic into `maybeDesugar(

[PATCH] D151190: [clangd] Do not end inactiveRegions range at position 0 of line

2023-06-05 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8ec44987e54b: [clangd] Do not end inactiveRegions range at position 0 of line (authored by nridge). Changed prior to commit: https://reviews.llvm.org/D151190?vs=528190&id=528304#toc Repository: rG LL

[PATCH] D125863: [clangd] Dont mark terminating PP-directives as skipped

2023-06-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. I believe this was superseded by D151190 , should we close this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125863/new/ https://reviews.llvm.org/D125863

[PATCH] D151785: [clangd] Desugar dependent type aliases for auto type hints

2023-06-07 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:267 StructuredBindingPolicy = TypeHintPolicy; StructuredBindingPolicy.PrintCanonicalTypes = true; } zyounan wrote: > nridge wrote: > > zyounan wrote: > > > `PrintCanoni

[PATCH] D152500: [clangd] Handle alias template in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. Herald added subscribers: jeroen.dobbelaere, kadircet, arphaman. Herald added a project: All. nridge updated this revision to Diff 530279. nridge added a comment. nridge published this revision for review. nridge added reviewers: hokein, sammccall. Herald added subscri

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: hokein, 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. Re

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Depends on D152500 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152645/new/ https://reviews.llvm.org/D152645 ___ cfe-commits mailing list cfe-c

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/HeuristicResolver.cpp:38 + if (const auto *TD = dyn_cast(Decls[0])) { +return Ctx.getTypeDeclType(TD).getTypePtr(); + } I wanted to call out the change on this line which is easy to overlook

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

2023-06-13 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Comment at: clang-tools-extra/clangd/InlayHints.cpp:196 +// Determines if any intermediate type in desugaring QualType QT is of +// substituted template paramet

[PATCH] D152520: [clangd] Unify printing policy for type hints

2023-06-13 Thread Nathan Ridge via Phabricator via cfe-commits
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/D152520/new/ https://reviews.llvm.org/D152520 ___

[PATCH] D152500: [clangd] Unwrap type sugar in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-14 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG19c9af81b1c5: [clangd] Unwrap type sugar in HeuristicResolver::resolveTypeToRecordDecl() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

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

2023-06-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thank you Sam for the suggested performance improvements, and outlining a way forward. @qchateau, are you interested in updating the patch to implement some of the described optimizations / address other comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 532008. 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/D152645/new/ https://reviews.llvm.org/D152645 Files: clang-tools-e

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/HeuristicResolver.cpp:56 + if (const auto *DNT = T->getAs()) { +T = resolveDeclsToType(resolveDependentNameType(DNT)); +if (!T) hokein wrote: > Is the `resolveDeclsToType` call necessary

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 532263. nridge marked an inline comment as done. nridge added a comment. Address last comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152645/new/ https://reviews.llvm.org/D152645 Files: clang-tools-extr

[PATCH] D152645: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl()

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a709689bc17: [clangd] Handle DependentNameType in HeuristicResolver::resolveTypeToRecordDecl… (authored by nridge). Repository: rG LLVM Github Mo

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

2023-06-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. > The major problem I see now is that we can't handle third party modules. > Third party modules refer to the modules whose source codes are not in > current project. The users are still able to see the hint from clangd if the > BMI of the third party modules are built a

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

2022-12-08 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D136594#3973908 , @ckandeler wrote: > In D136594#3973812 , @sammccall > wrote: > >> For my part, I still need to understand why we want the >> `builtin`/`UserModified` modifier. (The `

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

2022-12-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. While we're waiting for Sam to weigh in on the modifier question, a couple of comments about the details of the patch: - Does the implementation handle explicit operator calls, e.g. `a.operator+(b)`? If so, it would be nice to add some test coverage. - I see the patch ha

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

2022-12-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge resigned from this revision. nridge added a comment. In D138546#3980350 , @cpsauer wrote: > @nridge, I took a shot at amending the test. Thanks for the pointer! Please > me know if that looks good to you! The test changes look good, thanks. I'm

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

2022-12-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks, Christian, for your work on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136594/new/ https://reviews.llvm.org/D136594 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D139998: [clangd] Remove ReferenceFinder::Reference::Target

2022-12-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: tom-anders, 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. No one w

[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

2022-12-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge abandoned this revision. nridge added a comment. Abandoning in favour of D136594 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119077/new/ https://reviews.llvm.org/D119077 ___ cfe-commits mailing list

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

2022-12-14 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1426 + LookupRequest ContainerLookup; + llvm::DenseMap RefIndexForContainer; Index->relations(OverriddenBy, [&](const SymbolID &Subject, Unlike the QueryIndex case, I th

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

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Sorry for being a slow reviewer. I think showing type hints for `decltype(expr)` would be a nice enhancement. @v1nh1shungry, are you interested in working further on this? One high-level thought I had is: what if we attached the type hint to the closing `)` of the declt

[PATCH] D140191: [CodeComplete] Offer completions for headers with extension .hxx in include directives

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added reviewers: kadircet, sammccall. Herald added a project: All. nridge requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/1379 Repository:

[PATCH] D140191: [CodeComplete] Offer completions for headers with extension .hxx in include directives

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Precedent for treating `.hxx` as a supported header extension: https://reviews.llvm.org/D81366 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140191/new/ https://reviews.llvm.org/D140191

[PATCH] D140191: [CodeComplete] Offer completions for headers with extension .hxx in include directives

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 483616. nridge added a comment. Update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140191/new/ https://reviews.llvm.org/D140191 Files: clang/lib/Sema/SemaCodeComplete.cpp clang/test/CodeCompletion

[PATCH] D140191: [CodeComplete] Offer completions for headers with extension .hxx in include directives

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:10020 // Only files that really look like headers. (Except in special dirs). // Header extensions from Types.def, which we can't depend on here. const bool IsHeader = Filename

[PATCH] D140191: [CodeComplete] Offer completions for headers with extension .hxx in include directives

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG556d48fbaa99: [CodeComplete] Offer completions for headers with extension .hxx in include… (authored by nridge). Repository: rG LLVM Github Monore

[PATCH] D139998: [clangd] Remove ReferenceFinder::Reference::Target

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 483623. nridge added a comment. address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139998/new/ https://reviews.llvm.org/D139998 Files: clang-tools-extra/clangd/XRefs.cpp Index: clang-tools-extra/cla

[PATCH] D139998: [clangd] Remove ReferenceFinder::Reference::Target

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG40d29c0a714d: [clangd] Remove ReferenceFinder::Reference::Target (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

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

2022-12-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:1155 +// FIXME: Nice to show `: int &` +decltype((i)) $a[[a]] = i; + )cpp", v1nh1shungry wrote: > The only concern I have is how to deal with this situation.

[PATCH] D117463: [clangd] Disable expand-auto action on constrained auto.

2022-01-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D117463#3247915 , @sammccall wrote: > My intuition is that constrained auto is unlikely to be used in deducible > contexts, but maybe some people will like `Iterator auto I = > foo.begin()` or so... I expect this to be fairly

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

2022-01-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Thanks, I'm happy to try the array designators and see how they work out in practice. The issue of hints appearing and disappearing as you type a function call is one I've noticed, but I've g

[PATCH] D113874: [clang] Propagate requires-clause from constructor template to implicit deduction guide

2022-01-31 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. nridge updated this revision to Diff 404437. nridge added a comment. nridge retitled this revision from "[WIP] [clang] Attempt to fix crash in https://github.com/clangd/clangd/issues/890"; to "[clang] Propagate

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

2022-02-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:545 + // TemplateTypeParmTypes for implicit TTPs, instead of AutoTypes. + // Also we don't look very hard, just stripping const, references, pointers. + static TemplateTypeParmTypeLoc findContainedAutoT

[PATCH] D120258: [clangd] Add inlay hints for auto-typed parameters with one instantiation.

2022-02-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks, this is a nice extension of D119537 . I don't have a good sense for how common the "multiple instantiations, same type" scenario is; perhaps @Trass3r has an opinion on that. Comment at: clang-tools-extra/clangd

[PATCH] D120258: [clangd] Add inlay hints for auto-typed parameters with one instantiation.

2022-02-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:323 + "Instantiated function has fewer (non-pack) parameters?"); +return InstantiatedFunction->getParamDecl(ParamIdx); + } Here's a test case which slips past these c

[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

2022-02-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Still haven't had a chance to take a super detailed look, but a few high-level thoughts: 1. The AST nodes that reference operator names should store source ranges associated with the operator names, such that we shouldn't need to do token-by-token manipulation. For exam

[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

2022-02-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D119077#3337107 , @nridge wrote: > 1. The AST nodes that reference operator names should store source ranges > associated with the operator names, such that we shouldn't need to do > token-by-token manipulation. For example,

[PATCH] D120258: [clangd] Add inlay hints for auto-typed parameters with one instantiation.

2022-02-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120258/new/ https://reviews.llvm.org/D120258 __

[PATCH] D120416: [clangd] Function return type hints: support lambdas, don't duplicate "->"

2022-02-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision. nridge added a comment. This revision is now accepted and ready to land. Nice! Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:539 + // Lambda return hint shown even if no param list. + assertTypeHints("auto $L[[x]] = <:$ret[[:

[PATCH] D120498: [AST] Test RecursiveASTVisitor's current traversal of templates.

2022-02-27 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. will leave approval to hokein, but LGTM Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:172 - /// Return whether this visitor should recurse into - /// template instantiations. + /// Return whether we should traverse of AST nodes that were

[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

2022-02-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D119077#3339812 , @iannisdezwart wrote: > About your comment on the implicit calls to `operator[]` and `operator()`, > this was also a workaround. Do you have any idea how to get both parentheses > from a single `DeclRefExpr`?

[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

2022-02-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge requested changes to this revision. nridge added a comment. This revision now requires changes to proceed. One final note: many of the methods I've suggested produce a `SourceRange`, and we'll need to convert it to `Range`. I think it should be fine to convert the begin and end `SourceLoc

[PATCH] D94420: [clang-tooling] Prevent llvm::fatal_error on invalid CLI option

2022-02-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Herald added a project: clang-tools-extra. It would be nice to update the code example in the LibTooling documentation as well Repository: rG LLVM Github Monorepo C

[PATCH] D99052: [clangd] Produce semantic token for name referring to UnresolvedUsingValueDecl

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 340381. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99052/new/ https://reviews.llvm.org/D99052 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp clang

[PATCH] D99056: [clangd] Use HeuristicResolver to produce a better semantic token for name referring to UnresolvedUsingValueDecl

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 340382. nridge added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99056/new/ https://reviews.llvm.org/D99056 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp clang-tools-extra/cla

[PATCH] D99052: [clangd] Produce semantic token for name referring to UnresolvedUsingValueDecl

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGddfe13e757cb: [clangd] Produce semantic token for name referring to UnresolvedUsingValueDecl (authored by nridge). Repository: rG LLVM Github Mono

[PATCH] D99056: [clangd] Use HeuristicResolver to produce a better semantic token for name referring to UnresolvedUsingValueDecl

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd941863de2be: [clangd] Use HeuristicResolver to produce a better semantic token for name… (authored by nridge). Changed prior to commit: https://r

[PATCH] D100715: [clangd] Omit parameter hint if parameter name comment is present

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 340391. 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/D100715/new/ https://reviews.llvm.org/D100715 Files: clang-tools-e

[PATCH] D100731: [clangd] Omit parameter hint for setter functions

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 340392. 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/D100731/new/ https://reviews.llvm.org/D100731 Files: clang-tools-e

[PATCH] D100741: [clangd] Rename HeuristicResolver::resolveCallExpr() to resolveReturnTypeOfCallExpr()

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 340393. nridge marked an inline comment as done. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100741/new/ https://reviews.llvm.org/D100741 Files: clang-tools-ex

[PATCH] D100715: [clangd] Omit parameter hint if parameter name comment is present

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG753b247d71d7: [clangd] Omit parameter hint if parameter name comment is present (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D100731: [clangd] Omit parameter hint for setter functions

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f6cf2da8d94: [clangd] Omit parameter hint for setter functions (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D100741: [clangd] Rename HeuristicResolver::resolveCallExpr() to resolveReturnTypeOfCallExpr()

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc624e701491c: [clangd] Rename HeuristicResolver::resolveCallExpr() to resolveTypeOfCallExpr() (authored by nridge). Repository: rG LLVM Github Mo

[PATCH] D101270: [clangd] Avoid including HeuristicResolver.h from ParsedAST.h

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Githu

[PATCH] D101270: [clangd] Avoid including HeuristicResolver.h from ParsedAST.h

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Motivation: ParsedAST.h is widely included, and now that ParsedAST stores HeuristicResolved by unique_ptr, the header dependency is no longer necessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101270/new/ https://revi

[PATCH] D101275: [clangd] Hide inlay hints capability behind a command-line flag

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Githu

[PATCH] D100742: [clangd] Parameter hints for dependent calls

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 340439. nridge added a comment. Herald added a subscriber: cfe-commits. Add some more test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100742/new/ https://reviews.llvm.org/D100742 Files: clang-tools-e

[PATCH] D100742: [clangd] Parameter hints for dependent calls

2021-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a reviewer: sammccall. nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:264 +// FIXME: This one does not work yet. +A::static_member($par3[[t]]); } This is an interesting case. Cla

[PATCH] D101270: [clangd] Avoid including HeuristicResolver.h from ParsedAST.h

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG43cbf2bb84eb: [clangd] Avoid including HeuristicResolver.h from ParsedAST.h (authored by nridge). Changed prior to commit: https://reviews.llvm.or

[PATCH] D101275: [clangd] Hide inlay hints capability behind a command-line flag

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 342310. nridge marked 4 inline comments as done. nridge added a comment. Address review comments, including making the flag not hidden Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101275/new/ https://reviews.ll

[PATCH] D101275: [clangd] Hide inlay hints capability behind a command-line flag

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcea736e5b8a4: [clangd] Hide inlay hints capability behind a command-line flag (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D101740: [clangd] Fix test failure in initialize-params.test

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monorepo https://reviews.llvm.org

[PATCH] D101740: [clangd] Fix test failure in initialize-params.test

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Broke this in D101275 which put the capability behind a flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101740/new/ https://reviews.llvm.org/D101740 __

[PATCH] D101740: [clangd] Fix test failure in initialize-params.test

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3504e50b6db5: [clangd] Fix test failure in initialize-params.test (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D100742: [clangd] Parameter hints for dependent calls

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 342314. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100742/new/ https://reviews.llvm.org/D100742 Files: clang-tools-extra/clangd/HeuristicResolver.cpp clang

[PATCH] D100742: [clangd] Parameter hints for dependent calls

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:264 +// FIXME: This one does not work yet. +A::static_member($par3[[t]]); } sammccall wrote: > nridge wrote: > > This is an interesting case. Cl

[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Githu

[PATCH] D100742: [clangd] Parameter hints for dependent calls

2021-05-02 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1f8963c80195: [clangd] Parameter hints for dependent calls (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D102148: [clangd] Type hints for variables with 'auto' type

2021-05-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. nridge added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Githu

[PATCH] D102148: [clangd] Type hints for variables with 'auto' type

2021-05-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 343963. nridge added a comment. test formatting fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102148/new/ https://reviews.llvm.org/D102148 Files: clang-tools-extra/clangd/InlayHints.cpp clang-tools-extr

[PATCH] D102148: [clangd] Type hints for variables with 'auto' type

2021-05-09 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Here's a first draft of an implementation of type hints. It provided hints for variable declarations that use `auto` (possibly with const, &, etc.) as the type. There are additional places in which type hints could be useful, such as: - Functions with deduced return typ

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

2022-01-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks, this is a pretty nice addition! My only piece of high-level feedback is probing if the array element designators (`[0]=`) are useful enough to be worth the space/noise. The rest is minor implementation comments. Comment at: clang-tools-extra/c

[PATCH] D115425: [clangd] Generate ConfigFragment/YAML/docs from one tablegen source

2021-12-13 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. The patch is failing to build for me locally, with `clang-tools-extra/clangd/ConfigFragment.h:100:10: fatal error: 'ConfigFragment.gen.h' file not found`. I did re-run cmake and the error persists. Is there another step I'm missing? Repository: rG LLVM Github Monorep

[PATCH] D115425: [clangd] Generate ConfigFragment/YAML/docs from one tablegen source

2021-12-13 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D115425#3190603 , @sammccall wrote: > I had a discussion with @kadircet about this, we're not sure whether it > factors out enough to be better/simpler overall. (Hand-written ConfigFragment > + ConfigYAML + docs, vs table + ge

[PATCH] D115425: [clangd] Generate ConfigFragment/YAML/docs from one tablegen source

2021-12-13 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D115425#3190698 , @sammccall wrote: >> there's perhaps a matter of principle where clangd should try to avoid using >> code patterns (here, non-self-contained files) in its implementation code >> that it does not support > > A

[PATCH] D94382: [clangd] Avoid recusion in TargetFinder::add()

2021-01-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. nridge requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/633 Repository: rG LLVM G

[PATCH] D94382: [clangd] Avoid recusion in TargetFinder::add()

2021-01-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:790 +TEST_F(TargetDeclTest, TypedefCascade) { + Code = R"cpp( This test case is not strictly related to the bug, but it adds test coverage for a scenario that I t

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

2021-01-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:333 Decls; + const Decl *CurrentlyProcessing = nullptr; RelSet Flags; njames93 wrote: > Whats the purpose in tracking this? Is th

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

2021-01-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 315689. nridge marked an inline comment as done. nridge added a comment. Use map lookup instead of iteration Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94382/new/ https://reviews.llvm.org/D94382 Files: cla

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

2021-01-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:379 if (const TypedefNameDecl *TND = dyn_cast(D)) { add(TND->getUnderlyingType(), Flags | Rel::Underlying); Flags |= Rel::Alias; // continue with the alias. kadir

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

2021-01-10 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks for working on this! I haven't looked at the whole patch in detail, but I looked at some parts (mainly the `SymbolIndex` API and what in-memory structures we store to implement it) and wrote a few thoughts. Comment at: clang-tools-extra/clangd/

[PATCH] D92290: [clangd] Use clangd's Context mechanism to make the ASTContext of the AST being operated on available everywhere

2021-01-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision. nridge added a comment. That all sounds reasonable -- thanks for the suggestions! I've been meaning to factor out the heuristic resolution stuff into its own file anyways, this patch is as good a time as any to do it. Repository: rG LLVM Github Monore

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

2021-01-11 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 316000. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94382/new/ https://reviews.llvm.org/D94382 Files: clang-tools-extra/clangd/FindTarget.cpp clang-tools-ex

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

2021-01-12 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. I think that, as with incoming calls, the incremental value is in seeing the tree at a glance. So, you might query the outgoing calls for a function, expand the tree, and look over the transitive callees to see if the function ends up performing a certain type of operati

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

2021-01-12 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4718ec01669b: [clangd] Avoid recursion in TargetFinder::add() (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94382/new/ https://reviews

[PATCH] D94785: [clangd] Index local classes, virtual and overriding methods.

2021-01-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D94785#2501806 , @usaxena95 wrote: > Although now that I enabled this in all SymbolCollectorTests, this causes a > regression in RefContainers. > > void f2() { > (void) $ref1a[[f1]](1); > auto fptr = &$ref1b[[f

[PATCH] D92290: [clangd] Use clangd's Context mechanism to make the ASTContext of the AST being operated on available everywhere

2021-01-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 317272. nridge added a comment. Herald added a subscriber: mgorny. Change approach to factor out a HeuristicResolver class I didn't make it an optional parameter to allTargetDecls() etc. If you feel there are call sites which would benefit from //not// passin

[PATCH] D92290: [clangd] Factor out the heuristic resolver code into its own class

2021-01-18 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks for the comments! I'll have a more detailed look later, but just wanted to ask one clarifying question for now: Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:90 +for (const auto &Entry : + allTargetDecls(N->ASTNod

[PATCH] D95031: [clangd] Log warning when using legacy (theia) semantic highlighting.

2021-01-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. Thanks for the heads up! It would be nice to get D77702 landed before the removal of `TheiaSemanticHighlighting`. I'll try to find some time to rebase it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D92290: [clangd] Factor out the heuristic resolver code into its own class

2021-01-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:90 +for (const auto &Entry : + allTargetDecls(N->ASTNode, AST.getHeuristicResolver())) ActualDecls.emplace_back(Entry.first, Entry.second); sammcca

<    8   9   10   11   12   13   14   >