Repository navigation
feat(inlay hints): designators, dependent calls, config, semantics walk - #565
Merged
Merged
Conversation
…n hints - Designator hints via clang-tidy's getUnwrittenDesignators (the shipped prebuilt already links clangTidyUtils). - Dependent calls resolve through the unit's TemplateResolver; only a unique arity-viable candidate names parameters. - std::move/forward suppression now also matches by name in std, since -ffreestanding compiles carry no library-builtin IDs. - VisitCallExpr no longer drops default-argument hints when parameter hints are disabled.
Options resolve from workspace config per query in forward_query and travel inside QueryParams, replacing the hardcoded defaults in the stateful worker. Defaults come from feature::InlayHintsOptions itself so config and feature can never disagree.
38 per-topic fixtures replace the four seed files; fixture headers generate docs/en/features/inlay-hints.md. Unit tests keep only the option-gated categories (block-end, default arguments) the default-option snap paths cannot reach.
- The freestanding name fallback in is_simple_builtin now requires a single parameter, so the three-argument std::move algorithm keeps its hints (pinned in param_setters_builtins). - Dependent member calls no longer drop their first argument when the resolved candidate uses an explicit object parameter. - Reword the dependent-calls fixture header to keep internal machinery out of the generated doc. - New unit coverage: enabled=false gate, freestanding std::forward suppression, default arguments with parameters disabled.
📝 WalkthroughWalkthroughThe change adds configurable inlay-hint options, propagates them through worker queries, expands parameter and designator resolution, and adds fixture-based coverage for parameter, type, default-argument, and aggregate hints. Documentation is now generated from fixtures. ChangesInlay hints
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab8835ea12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/en/guide/configuration.md`:
- Line 135: Clarify the `[inlay_hints]` documentation by stating that edits to
`clice.toml` take effect only after restarting the server, after which a
client-side refresh is sufficient to apply the per-request options. Remove the
ambiguity between client refresh and server configuration reload.
In `@tests/snap/inlay_hint/param_explicit_instantiation.cpp`:
- Line 12: Update the explicit instantiation in the fixture to an
explicit-instantiation declaration by adding the required extern specifier to
template apply<int>. Preserve the existing signature and test scenario for
duplicate hints.
In `@tests/snap/inlay_hint/param_names.cpp`:
- Around line 10-12: Update the Point fixture to test an actual move-constructor
call: add an explicit Point(Point&&) declaration/definition and construct the
target from an xvalue such as a moved Point instance. Ensure the test no longer
relies on C++17 prvalue elision, so move-constructor hint suppression is
exercised.
In `@tests/snap/inlay_hint/type_bindings_tuple.cpp`:
- Around line 13-31: Replace the manually declared std::tuple_size and
std::tuple_element primary templates with the appropriate standard library
header inclusion, while retaining only the IntPair specializations and their
existing values/types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0dc4a832-015d-4a13-928d-9cd377c9ac9c
📒 Files selected for processing (97)
docs/en/features/inlay-hints.mddocs/en/guide/configuration.mdsrc/feature/inlay_hints.cppsrc/server/compiler/compiler.cppsrc/server/protocol/worker.hsrc/server/state/config.cppsrc/server/state/config.hsrc/server/worker/stateful_worker.cpptests/integration/lifecycle/config.test.tstests/snap/inlay_hint/basic.cpptests/snap/inlay_hint/basic.snap.ymltests/snap/inlay_hint/conversion_hints.cpptests/snap/inlay_hint/ctad_arguments.cpptests/snap/inlay_hint/designator_aggregates_only.cpptests/snap/inlay_hint/designator_aggregates_only.snap.ymltests/snap/inlay_hint/designator_anonymous.cpptests/snap/inlay_hint/designator_anonymous.snap.ymltests/snap/inlay_hint/designator_basic.cpptests/snap/inlay_hint/designator_basic.snap.ymltests/snap/inlay_hint/designator_nested.cpptests/snap/inlay_hint/designator_nested.snap.ymltests/snap/inlay_hint/designator_parenthesized.cpptests/snap/inlay_hint/designator_recovery.cpptests/snap/inlay_hint/designator_recovery.snap.ymltests/snap/inlay_hint/designator_suppression.cpptests/snap/inlay_hint/designator_suppression.snap.ymltests/snap/inlay_hint/designators.cpptests/snap/inlay_hint/designators.snap.ymltests/snap/inlay_hint/param_anonymous.cpptests/snap/inlay_hint/param_anonymous.snap.ymltests/snap/inlay_hint/param_case_insensitive.cpptests/snap/inlay_hint/param_case_insensitive.snap.ymltests/snap/inlay_hint/param_deducing_this.cpptests/snap/inlay_hint/param_deducing_this.snap.ymltests/snap/inlay_hint/param_definition_names.cpptests/snap/inlay_hint/param_definition_names.snap.ymltests/snap/inlay_hint/param_dependent.cpptests/snap/inlay_hint/param_dependent.snap.ymltests/snap/inlay_hint/param_explicit_instantiation.cpptests/snap/inlay_hint/param_explicit_instantiation.snap.ymltests/snap/inlay_hint/param_forwarding.cpptests/snap/inlay_hint/param_forwarding.snap.ymltests/snap/inlay_hint/param_function_objects.cpptests/snap/inlay_hint/param_function_objects.snap.ymltests/snap/inlay_hint/param_hints.cpptests/snap/inlay_hint/param_hints.snap.ymltests/snap/inlay_hint/param_implicit_conversions.cpptests/snap/inlay_hint/param_implicit_conversions.snap.ymltests/snap/inlay_hint/param_inherited_constructors.cpptests/snap/inlay_hint/param_inherited_constructors.snap.ymltests/snap/inlay_hint/param_macros.cpptests/snap/inlay_hint/param_macros.snap.ymltests/snap/inlay_hint/param_names.cpptests/snap/inlay_hint/param_names.snap.ymltests/snap/inlay_hint/param_operators.cpptests/snap/inlay_hint/param_operators.snap.ymltests/snap/inlay_hint/param_pack_constructors.cpptests/snap/inlay_hint/param_pack_constructors.snap.ymltests/snap/inlay_hint/param_packs.cpptests/snap/inlay_hint/param_packs.snap.ymltests/snap/inlay_hint/param_pseudo_objects.cpptests/snap/inlay_hint/param_pseudo_objects.snap.ymltests/snap/inlay_hint/param_references.cpptests/snap/inlay_hint/param_references.snap.ymltests/snap/inlay_hint/param_setters_builtins.cpptests/snap/inlay_hint/param_setters_builtins.snap.ymltests/snap/inlay_hint/param_suppression.cpptests/snap/inlay_hint/param_suppression.snap.ymltests/snap/inlay_hint/template_parameter_hints.cpptests/snap/inlay_hint/type_auto.cpptests/snap/inlay_hint/type_auto.snap.ymltests/snap/inlay_hint/type_auto_params.cpptests/snap/inlay_hint/type_auto_params.snap.ymltests/snap/inlay_hint/type_auto_return.cpptests/snap/inlay_hint/type_auto_return.snap.ymltests/snap/inlay_hint/type_bindings_tuple.cpptests/snap/inlay_hint/type_bindings_tuple.snap.ymltests/snap/inlay_hint/type_decltype.cpptests/snap/inlay_hint/type_decltype.snap.ymltests/snap/inlay_hint/type_dependent.cpptests/snap/inlay_hint/type_dependent.snap.ymltests/snap/inlay_hint/type_explicit_source.cpptests/snap/inlay_hint/type_explicit_source.snap.ymltests/snap/inlay_hint/type_hints.cpptests/snap/inlay_hint/type_hints.snap.ymltests/snap/inlay_hint/type_lambdas.cpptests/snap/inlay_hint/type_lambdas.snap.ymltests/snap/inlay_hint/type_scopes.cpptests/snap/inlay_hint/type_scopes.snap.ymltests/snap/inlay_hint/type_structured_bindings.cpptests/snap/inlay_hint/type_structured_bindings.snap.ymltests/snap/inlay_hint/type_sugar.cpptests/snap/inlay_hint/type_sugar.snap.ymltests/unit/feature/inlay_hint_tests.cpptests/unit/server/config_tests.cpptools/feature_docs.tstools/snap/standalone.ts
💤 Files with no reviewable changes (8)
- tests/snap/inlay_hint/designators.cpp
- tests/snap/inlay_hint/basic.cpp
- tests/snap/inlay_hint/designators.snap.yml
- tests/snap/inlay_hint/type_hints.snap.yml
- tests/snap/inlay_hint/type_hints.cpp
- tests/snap/inlay_hint/basic.snap.yml
- tests/snap/inlay_hint/param_hints.cpp
- tests/snap/inlay_hint/param_hints.snap.yml
The builtin-ID switch plus name fallback approximates "too common to hint"; a curated table of qualified names and signature shapes is the explicit form to grow toward.
- Default-argument hints now treat type_name_limit = 0 as unlimited, matching type hints and the documented contract. - param_names gains a real move constructor exercised via an xvalue; the previous case only covered copy plus an elided prvalue. - Reword the explicit-instantiation fixture title (definition, not declaration) and the configuration-effect wording in both docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9072bc4481
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace the RecursiveASTVisitor traversal with a single Collector class walking unit.semantics().node_entries(), aligning with semantic tokens. The pre-order table drives dispatch; subtree skipping covers the MS property assignment case, and semantic-form accessor calls of pseudo object expressions are derived from the recorded wrapper on demand. The outermost init list records its semantic form, so designator handling normalizes back to the syntactic form. Pin default member initializer parameter hints in the operators fixture.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/snap/inlay_hint/param_operators.snap.yml`:
- Around line 5-8: Update the param_operators snapshot entries to match the
fixture’s shifted line numbers: remove the stale hint at 17:16 and move the
hints for S defaulted{3}; and Holder() : member(42) {} to their regenerated
positions, preserving their exact columns and hint metadata.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bcca9b9-d301-4288-a076-4b6a6551c360
📒 Files selected for processing (4)
docs/en/features/inlay-hints.mdsrc/feature/inlay_hints.cpptests/snap/inlay_hint/param_operators.cpptests/snap/inlay_hint/param_operators.snap.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/en/features/inlay-hints.md
16bit-ykiko
changed the title
16bit-ykiko
added a commit
that referenced
this pull request
Aug 1, 2026
…ics walk (#566) Third per-feature round after semantic tokens (#564) and inlay hints (#565): document symbols gets its traversal migrated onto the `Semantics` node table, four outline bugs fixed (plus one table-level bug in the builder), and its snap corpus rewoven into 24 doc-generating fixtures. ## Traversal migrated onto the semantics node table `document_symbols.cpp` no longer runs a `FilteredASTVisitor` over the TU. A single `Collector` walks the cached `unit.semantics().node_entries()` — the DFS pre-order record of the interested file's written AST. Nesting comes for free: a symbol's frame stays open while the walk index is inside its `subtree_end`, so the outline tree is rebuilt with a plain `(subtree_end, cursor)` stack, and implicit instantiations are skipped as an index jump. The migration was proven equivalent against the pre-existing snapshots and full unit suite before any behavior was changed. One behavior improvement falls out of the table's recording contract: abbreviated function templates (`void f(Concept auto x)`) used to vanish from the outline entirely — the written `FunctionDecl` hides inside an implicit `FunctionTemplateDecl`, which the old visitor skipped wholesale. The builder records written children of implicit decls, so these functions now appear (pinned in `kinds_templates`). ## Table-level fix: structured bindings recorded twice Namespace-scope `BindingDecl`s are members of the enclosing `DeclContext` *and* explicitly traversed by `TraverseDecompositionDecl`'s bindings loop, so the builder recorded each one twice — every table consumer saw doubles (the outline showed each binding twice). The builder now records a `BindingDecl` only when its walk parent is its own `DecompositionDecl`. Block-scope and TU-scope bindings were already single-visit; both are pinned. ## Outline fixes - **Template specializations**: explicit and partial specializations of class and variable templates never appeared (`is_interested` lacked their decl kinds); their members were orphaned at namespace level. Now `Box<void>`, `Box<T*>`, `pi<int>`, `pi<T*>` appear with members correctly nested. - **Type aliases**: `typedef`, `using` aliases and alias templates never appeared at all. They now render with a `type alias` detail, mapped to LSP `Class` (matching clangd). - **Multi-token name selection ranges**: `~Widget` selected only the `~`; `operator==` and `operator bool` selected only `operator`. Selection ranges now come from `DeclarationNameInfo` and cover the full written name. - **Names spelled in macro arguments** (clangd#1941): `VAR(name)` selected the macro name instead of `name`. The name range now goes through `getFileLoc`, so argument-spelled names select their written spelling while body-spelled names keep the invocation site; the symbol range is widened when needed to preserve the LSP range ⊇ selection-range invariant. ## Corpus rewoven: 24 fixtures, doc generated from them The 4 seed fixtures were replaced by 24 itemized fixtures with `///` doc headers; `document_symbol` is registered in `feature_docs.ts` and the checklist sections of `docs/en/features/document-symbols.md` are now generated from the corpus. Probes confirmed several checklist items are supported by construction and are now pinned: default-argument stripping (clangd#221), multiline signature ranges (clangd#2221), macro-expansion symbol locations (clangd#475), friend function definitions, local symbols inside function bodies (clangd#616), and UTF-16 column counting (CJK fixture). Unsupported items (access-specifier grouping clangd#499, base-class detail, macro/include/module/`#pragma mark` outline entries, symbol tags clangd#2123) are recorded as compiled-out stubs. The count-based unit tests are retired; every case they touched is pinned structurally by a snapshot, including implicit instantiations *not* appearing. ## Verification All four suites green: unit (1168) and snap (280) on both RelWithDebInfo and Debug (LLVM assertions), integration (336), smoke (3/3), `npm run check`, `feature_docs.ts check`. Three-way pre-PR review (correctness / style / test coverage); findings addressed: inline comment style, reuse of `decls::is_implicit_instantiation`, and three added pins (implicit instantiation absence, block-scope bindings, static data member + named nested struct).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues the per-feature test-weaving campaign (#564): inlay hints get their snap corpus, fixture-generated docs, and — since the tests exist to be acted on — fixes for everything they surfaced, plus the configuration section the feature never had. With the corpus in place as a safety net, the traversal itself then moves onto the shared semantics node table.
Feature fixes
Point{1, 2}renders.x=/.y=, nested aggregates flatten (.b.x=), anonymous unions/structs vanish from the path,/*name=*/comments and written designators suppress, and broken initializers keep their surviving designators. UsesgetUnwrittenDesignatorsfrom the clang-tidy utils the prebuilt LLVM already ships.apply(value),holder.member(value),Holder<T>::static_member(value)) resolve through the template resolver's arity-filtered candidate set; only a unique candidate names parameters, so ambiguous overloads stay bare instead of guessing.-ffreestandingstrips library-builtin IDs, which madestd::forward(x)grow a stray&hint. The suppression now also matches the cast-like std names, constrained to single-parameter functions so the three-argumentstd::movealgorithm keeps its hints.getCalleeDecl()is gone.[inlay_hints]configurationNew config section (clice.toml /
initializationOptions):enabled,parameters,deduced_types,designators,block_end,default_arguments,type_name_limit. Defaults come fromfeature::InlayHintsOptionsitself, so config and feature cannot disagree. The master resolves options from workspace config per query and ships them inQueryParams, replacing the hardcoded defaults in the stateful worker — block-end and default-argument hints were previously unreachable on the wire. Documented in the configuration guide; an integration test pins the end-to-end path with a default-config control.Snap corpus & docs
38 per-topic fixtures replace the four seed files; every active fixture is
snap: sharedbetween the standalone and wire paths. Fixture headers generatedocs/en/features/inlay-hints.md(parameter/type/designator sections plus unsupported stubs for CTAD, template-parameter and conversion hints); option-gated categories are documented by hand since the default-option snap paths cannot reach them. Unit tests shrink to what the corpus cannot express: block-end, default arguments, the option gates, and freestanding builtin suppression.Edge coverage migrated from the deleted unit cases before removal: decltype in every written position, tuple-protocol bindings, scope suppression, pack head/tail forwarding, explicit-instantiation dedup, macro call-site shapes, MS property pseudo-objects, deducing
this.Known gaps stay pinned as
partial/unsupportedfixtures: case-insensitive name matching (clangd#2248), inherited-constructor names (clangd#1364), dependentauto(clangd#2275), redundant hints on explicit casts (clangd#1749), parenthesized aggregates (clangd#2540).Traversal rewritten onto the semantics node table
The feature's RecursiveASTVisitor is gone: hints are collected by a single
Collectorwalking the unit's cachedSemanticsnode table — the same flat DFS pre-order record of the written AST that already serves selection and semantic tokens — so the traversal runs once per parse instead of once per request, and traversal policy lives in exactly one place. Subtree skipping becomes an index jump oversubtree_end(the MS-property assignment case included), and the old Builder/Visitor split collapses into the one class since nothing forces a CRTP visitor anymore.Two table contracts surfaced during the migration:
PseudoObjectExprrecords only its syntactic subtree; the accessor call of an MS property subscript read lives in the semantic forms and is derived on demand from the recorded wrapper, keeping therow:/column:hints.InitListExprenters the table as its semantic form (the pointer its AST parent stores). Designator handling normalizes back to the syntactic form — the visitor used to do that normalization for every consumer implicitly, and four designator fixtures caught the divergence byte-for-byte before it could ship.The corpus doubled as the equivalence net: every snapshot is byte-identical across the rewrite. One fixture extension pins parameter hints in default member initializers, a construct the corpus previously reached only through constructor-initializer lists.
Verification
All four suites green locally: unit 1176, integration 336 (new config round-trip test included), smoke 3/3, snap 256 (+50 intentional skips),
npm run checkclean; unit and snap additionally pass against the Debug build with assertions on. Two rounds of three-way pre-PR review (correctness / style / coverage) applied: over-matching builtin fallback, dependent explicit-object argument drop, doc machinery leak, and naming/coverage fixes on the table walk.