Skip to content

feat: show elaborated parameter value on hover#315

Open
wasinsangdam wants to merge 5 commits into
hudson-trading:mainfrom
wasinsangdam:main
Open

feat: show elaborated parameter value on hover#315
wasinsangdam wants to merge 5 commits into
hudson-trading:mainfrom
wasinsangdam:main

Conversation

@wasinsangdam

Copy link
Copy Markdown

Description:

Summary

Adds an elaborated Value: line to parameter hovers so users can see what a parameter actually resolves to in the loaded design, not just its declared default. This is especially useful for parameters that are passed down through several module layers via overrides.

The value is resolved server-side in two ways, in order of preference:

  1. Active instance — the VSCode Hierarchy View now reports the currently focused instance to the server via a new slang.setActiveInstance request. When the hovered parameter belongs to that instance's module, its instance-specific value is shown.
  2. Module-based fallback — when no active instance is set, or it does not match the hovered module, the server walks every instance of the module via moduleToInstances. If they all share the same value the hover shows it; otherwise the value is treated as ambiguous and omitted, leaving the existing default-value hover unchanged.

This means the feature works out of the box even without any client-side state: as soon as the design is elaborated, hovers on parameters of single-instance modules (or modules whose instances happen to agree) light up.

  • ServerCompilation::getElaboratedParamValue(moduleName, paramName, activeInstancePath?) encapsulates the lookup logic.
  • ServerDriver::getDocHover calls it for ParameterSymbol hovers and appends a Value: line when a value is returned.
  • New LSP request slang.setActiveInstance (string hierPath) lets the client push the focused hierarchical instance to the server. Stored on SlangServer::m_activeInstancePath.
  • VSCode extension: ProjectComponent calls setActiveInstance whenever the Hierarchy View focus changes.

Tests

  • HoverParameterElaboratedUniqueInstance — single-instance module, elaborated value surfaces without any active-instance state.
  • HoverParameterElaboratedAluModule — multi-instance ALU under CPU; edits cpu_testbench.sv to set DATA_WIDTH=40, saves to trigger comp->refresh(), and verifies that alu.sv's WIDTH hover shows 40 (not the declared default of `32).

All 947 assertions / 185 cases in server_unittests pass.

Notes for reviewers

  • The active-instance path is purely additive — clients that don't know about slang.setActiveInstance just get the module-based fallback behavior.
  • The elaborated value depends on the full design compilation, which only refreshes on didSave. This is worth keeping in mind when manually testing.

wasinsangdam and others added 5 commits April 19, 2026 16:34
When a design is loaded and the user clicks an instance in the Hierarchy
View, hovering on a parameter declaration in that module's source file now
shows the elaborated (override) value for that specific instance rather than
the shallow-compilation default.

Changes:
- Add slang.setActiveInstance server command to track the focused instance
- Thread active instance path from SlangServer through to getDocHover
- Add ServerCompilation::getInstanceParamValue to look up a parameter value
  from the elaborated AST using the same root.lookupName path the Hierarchy
  View already uses; a module-name guard prevents cross-module contamination
- Update getHover to use the elaborated value when available, falling back to
  the existing default-value display otherwise
- Notify the server from the VS Code extension when setInstance fires

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add param_leaf.sv / param_top.sv / param_test.f test fixtures:
  param_leaf declares DEPTH=4 (default), param_top overrides with DEPTH=16
- Fix DocumentHandle::getHoverAt to pass m_activeInstancePath through to
  getDocHover so the active instance is respected during tests
- Expose m_activeInstancePath via using-declaration in ServerHarness
- Add HoverParameterElaborated test: verifies default value without active
  instance, elaborated override value with active instance, and restoration
  of default after clearing the active instance

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Redesigned getElaboratedParamValue to not require client-side active
instance tracking. When active instance is known it is still preferred,
but otherwise the value is resolved from moduleToInstances: if every
instance of the module shares the same value the hover shows it,
otherwise the result is ambiguous and omitted.

Also removed the param_test fixture; coverage is now provided by two
cases reusing comp_repo (unique-instance and multi-instance ALU with
override via cpu_testbench edit+save).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat: show elaborated parameter value on hover
@AndrewNolte

AndrewNolte commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

I think it'd be good to have an active instance per module, rather than a single active instance. And there should be some indicators of the active instance-

  • The modules view should indicate which instance is active. I also think one should be selected by default- probably the first one / highest in the hierarchy.
  • An inlay hint after the module name, which is the selected hierarchical path.

Comment thread include/SlangServer.h
std::optional<waves::WcpClient> m_wcpClient = std::nullopt;

// The hierarchical path of the currently active instance, set via slang.setActiveInstance
std::optional<std::string> m_activeInstancePath;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a map and live in the server compilation

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Quick clarification on the inlay hint: should it be a standard LSP inlay hint rendered in the editor (e.g., virtual text after module alu in alu.sv showing the active instance path), or text added to the module label in the sidebar Modules view? Happy to implement either — just want to make sure I build the right one.

Will go ahead and implement the per-module active instance map and the default selection + indicator in the Modules view in the meantime.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I have a big refactor of the client/server api, since right now the source locations can get out of sync with any edits. It also touches a lot of the server compilation code, so let me just add this feature onto that pr. Thanks for giving this a shot though! Once that's more stable I think there will be a lot of interesting features to contribute there, like displaying signal values in the inlay hints over wcp.

When I was experimenting with it I found codelenses a lot better actually, since then it won't clip the instance path. I also added a status bar item to indicate the selected instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants