--allow-toplevel-iface-ports: Apply $static_assert iface param overrides - #1910
Conversation
This allows users to pin iface port params, allowing for better LSP features outside of a design context. The body will be inspected for $static_asserts, and will use those to fill in the expected parameter values for iface ports. stack-info: PR: MikePopoloski#1910, branch: AndrewNolte/stack/24
9929b41 to
9d1724b
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1910 +/- ##
==========================================
- Coverage 96.05% 96.01% -0.04%
==========================================
Files 247 247
Lines 56717 56855 +138
==========================================
+ Hits 54482 54592 +110
- Misses 2235 2263 +28
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This allows users to pin iface port params, allowing for better LSP features outside of a design context. The body will be inspected for $static_asserts, and will use those to fill in the expected parameter values for iface ports. Related issue: hudson-trading/slang-server#401 stack-info: PR: MikePopoloski#1910, branch: AndrewNolte/stack/24
9d1724b to
b4d7a64
Compare
26fd3fe to
d6ada10
Compare
This allows users to pin iface port params, allowing for better LSP features outside of a design context. The body will be inspected for $static_asserts, and will use those to fill in the expected parameter values for iface ports. Related issue: hudson-trading/slang-server#401 stack-info: PR: MikePopoloski#1910, branch: AndrewNolte/stack/24
| /// Useful for propagating an already-elaborated value (e.g. copied from another instance, or | ||
| /// derived from a constraint) onto the parameter of a freshly built instance. | ||
| void addValueOverride(std::string_view name, ConstantValue value) { | ||
| resolvedOverrides.emplace(name, std::move(value)); |
There was a problem hiding this comment.
These are stored as a variant and the caller already has a variant, so you may as well have one method that takes a variant and not force the caller to unwrap it.
There was a problem hiding this comment.
I had added these as values/types for a focused-instance selection feature, but I'll switch to one method here. I think we should be able to do that feature with cloning/recreating syntax nodes.
| // A pre-resolved type override takes precedence and is applied directly, since it has | ||
| // already been resolved and doesn't depend on any instance context. Localparams can't be | ||
| // overridden, so fall through to normal resolution for those. | ||
| if (auto it = resolvedOverrides.find(decl.name); |
There was a problem hiding this comment.
I don't love adding another map lookup for every param we create, when this will almost never be used. Can this extra value be stored alongside other stuff in the assignments map so we get it as part of the main lookup?
| SmallVector<IfaceParamConstraint> constraints; | ||
| SmallVector<IfaceParamOverride> paramOverrides; | ||
| if (bodySyntax) { | ||
| collectIfaceParamConstraints(ifacePort, *bodySyntax, constraints); |
There was a problem hiding this comment.
These two methods are only ever called back to back, so you can just make them one method instead, and avoid exposing the intermediary IfaceParamConstraint type.
| SmallVectorBase<IfaceParamConstraint>& constraints) { | ||
| auto& def = *port.interfaceDef; | ||
|
|
||
| for (auto member : topBody.members) { |
There was a problem hiding this comment.
For every iface port we loop over every body member. Probably this should be one loop to find all static asserts and then reusing them for each iface port.
d6ada10 to
70e1d10
Compare
This allows users to pin iface port params, allowing for better LSP features outside of a design context. The body will be inspected for $static_asserts, and will use those to fill in the expected parameter values for iface ports. Related issue: hudson-trading/slang-server#401 stack-info: PR: MikePopoloski#1910, branch: AndrewNolte/stack/24
41c582c to
6c9bf23
Compare
2 participants
Stacked PRs:
View individual changes
--allow-toplevel-iface-ports: Apply $static_assert iface param overrides
This allows users to pin iface port params, allowing for better LSP features outside of a design context.
The body will be inspected for $static_asserts, and will use those to fill in the expected parameter values for iface ports.
Related issue: hudson-trading/slang-server#401