Judge a block parameter by which parameter it is, not only by its value - #24
Merged
Conversation
A user clicked three links the Usage column had drawn and nothing happened. Two of the three rows should never have existed: a Bus Selector's `OutputSignals=a,b` and a Math Function's `Operator=square` are not references to data at all. The `<P>` gate was a property blocklist plus a value test, and neither can see the difference between `Gain=square` and `Operator=square` — `square` is a legal variable name, so the value test has to say yes. What rules it out is WHICH parameter holds it: `Operator` is an option list of fifteen choices Simulink itself enforces. That is a fact about the block type, which the gate never had. The cost was not a spare row. On a MATLAB-written model whose bus carries a signal `a` while the model workspace also holds a variable `a`, `Simulink.findVars` gives `a` no users at all — and this package put the Bus Selector on it. A false reverse edge, and one a reader cannot tell from a true one. So the gate now takes the block type and consults two measured tables: - `ENUM_BLOCK_PARAMS`, 343 (BlockType, parameter) pairs generated by the new `gen_block_params.m` from `DialogParameters` over every block in `simulink` and `simulink_extras`. A pair is emitted only when it is an option list on EVERY instance — `SubSystem|OutDataTypeStr` is free text on a plain subsystem and a menu on a masked one, and suppressing the free-text case would hide a real `Simulink.NumericType` reference. - a three-entry hand list of free-text parameters `findVars` measurably never credits: a Bus Selector's `OutputSignals`, a Bus Assignment's `AssignedSignals`, and a Model block's file name under all three of its spellings. Not derivable from any dialog property, so `probe_non_data_params.m` asks MATLAB directly, on models built so the answer is sharp. Keying on the pair rather than the parameter name is what makes suppression safe: `Operator` is a menu on a Math block and an expression on a Gain, and `Format`, `SimulateUsing` and `TriggerType` are each measured both ways. A block type in neither table — every toolbox block, every masked library link — is judged exactly as before, which is the direction issue #9 argues for: a spare row is a row a reader ignores, a suppressed reference is a Usage cell that lies by omission. Two rules were measured first and both are wrong, both by hiding real references: the `dont-eval` attribute (findVars credits a `dont-eval` SampleTime, so a 654-name blocklist from it would have deleted `SampleTime = Ts`) and "set_param refuses an identifier" (a successful set_param mutates the block, so the refusal count moves with scan order). Both are written out in the generator's header so they are not re-derived. Left deliberately admitted: a Model block's `ParameterArgumentValues`, one parameter away from the file name that is now dropped — a model argument is an expression in the parent's scope and exactly the kind of reference this column exists to show. `enumBlockParams.ts` is generated into `src/` rather than into a fixture directory, because the parser imports it at parse time; `drift.mjs` now regenerates and diffs it per block type, since a release that turned a menu into free text would leave the suite green and a reference hidden. Held by `blockParamUsages.test.ts` — 7 of its cases fail with the fix reverted — including the pair-keying itself, the mask parameters the gate must not reach, and the `.slx` and `.mdl` of the same three blocks held against EACH OTHER rather than against a literal, because one rule spelled in two parsers is the shape this repo's bugs keep taking. `usageIndex.test.ts` now states the better truth for a Model block: no parameter rows, and the reference still reported resolved off the same bytes.
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
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.
A user clicked three links the Usage column had drawn and nothing happened. Two of the
three rows should never have existed: a Bus Selector's
OutputSignals=a,band a MathFunction's
Operator=squareare not references to data at all. (The dead link is arenderer defect and is fixed in the VS Code repo; the rows are this package's.)
What was wrong
The
<P>gate was a property blocklist plus a value test, and neither can see thedifference between
Gain=squareandOperator=square—squareis a legal variablename, so the value test has to say yes. What rules it out is which parameter holds
it:
Operatoris an option list of fifteen choices Simulink itself enforces. That isa fact about the block type, which the gate never had.
The cost was not a spare row. On a MATLAB-written model whose bus carries a signal
awhile the model workspace also holds a variable
a,Simulink.findVarsgivesanousers at all — and this package put the Bus Selector on it. A false reverse edge,
and one a reader cannot tell from a true one.
The fix
isParamReferencenow takes the block type and consults two measured tables:ENUM_BLOCK_PARAMS— 343(BlockType, parameter)pairs generated by the newgen_block_params.mfromDialogParametersover every block insimulinkandsimulink_extras. A pair is emitted only when it is an option list on everyinstance:
SubSystem|OutDataTypeStris free text on a plain subsystem and a menu on amasked one, and suppressing the free-text case would hide a real
Simulink.NumericTypereference.findVarsmeasurably nevercredits — a Bus Selector's
OutputSignals, a Bus Assignment'sAssignedSignals, anda Model block's file name under all three spellings. Not derivable from any dialog
property, so
probe_non_data_params.masks MATLAB directly, on models built so theanswer is sharp.
Keying on the pair is what makes suppression safe:
Operatoris a menu on a Mathblock and an expression on a Gain. A block type in neither table — every toolbox block,
every masked library link — is judged exactly as before, which is the direction #9
argues for: a spare row is a row a reader ignores, a suppressed reference is a Usage
cell that lies by omission.
Two other rules were measured first and both are wrong, both by hiding real
references — the
dont-evalattribute, and "set_paramrefuses an identifier". Bothare written out in the generator's header so they are not re-derived.
enumBlockParams.tsis generated intosrc/rather than into a fixture directory,because the parser imports it at parse time;
drift.mjsnow regenerates and diffs itper block type, since a release that turned a menu into free text would leave the suite
green and a reference hidden.
Tests
blockParamUsages.test.ts— 7 of its cases fail with the fix reverted — covering thepair-keying itself, the mask parameters the gate must not reach, the parameters that
must stay admitted (
ParameterArgumentValues), and the.slxand.mdlof the samethree blocks held against each other rather than against a literal. Full suite:
4328 pass,
npm run verifygreen.