ArgParser: lift option and defaulting over records and structural unions - #613
Open
Smaug123 wants to merge 5 commits into
Open
ArgParser: lift option and defaulting over records and structural unions#613Smaug123 wants to merge 5 commits into
Smaug123 wants to merge 5 commits into
Conversation
The list arm of `createParseFunction` checked only for maps, while every sibling arm rejects each nesting it cannot express. So these two shapes classified successfully and died much later against an assertion phrased as an internal error -- which they are not, being reachable from ordinary source. The positional path has always rejected them properly. Rejecting in the shared arm means both paths now give the same message, and the positional arms which said so become unreachable, so they go. (The positional `List(Map)` arm was already unreachable: the shared arm has always rejected that one.) `Choice` stays permitted here: the positional path spells its before/after-`--` tag as `Choice<'a, 'a> list`, and on the ordinary path the field-level default-attribute checks reject the shape first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A field of type `SomeArgs option` now contributes a whole group of arguments which need not be supplied. Previously a record or union was recognised only as a field's exact outer type, so one wrapper layer sent the field to the leaf machinery, which cannot parse a structural type and gave up with "Could not decide how to parse arguments". `toParseSpec` now peels one container layer before asking whether what remains is structural, and wraps the tree it would have built anyway in a new `ParseTree.Container`. That erases to a two-case `Sum` whose second case is the empty product, so the runtime needs to know nothing about it: "which case was selected" already means "was anything beneath supplied". The container is a node of its own rather than a `Sum` because it is not an alternation the author wrote, and help text must not present it as one: it prints the ordinary group header with `(optional)`, and a union nested beneath keeps its own "exactly one of" framing. A payload which is itself satisfiable with no arguments is refused, since absence could not then be told from presence. That gate is what makes the erasure sound: the empty case can never be touched, and the payload is never empty-satisfiable, so exactly one case is always selectable and none of the runtime's three selection errors can arise. The synthetic case names are therefore unreachable in any message. The reference oracle could not previously generate a Sum case with no leaves at all -- `chooseCuts` gives every case at least one named id -- so that shape was untested despite being representable. Three properties now cover it, including one which confirms the gate is necessary rather than merely conservative: without it, the empty command line genuinely is ambiguous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Choice<SomeArgs, SomeArgs>` now works as `SomeArgs option` does, but says that omitting the group means a particular value rather than no value: `Choice2Of2` carries the default, `Choice1Of2` what was supplied, exactly as for a defaulted leaf. The default must come from `[<ArgumentDefaultFunction>]`. An attribute argument is a compile-time constant and there is no constant which is a record; an environment variable is one string, and no spelling turns one string into a whole group of arguments. Both are refused by name rather than being ignored. The container kind is decided lazily, after the core type is known to be structural. `Choice<int, int>` peels here too, and must reach the leaf machinery with its own diagnostics intact rather than being told it needs a static member returning a group. A defaulted group's value is not rendered in help text. There is no single token which supplies a whole group, so `renderLeafValue` has nothing to render; the help says only that a default exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Optional and defaulted groups nest, and [<ArgumentPrefix>] namespaces the whole subtree through the wrapper. Both fell out of the design rather than being built for, so they are pinned by tests rather than left to be rediscovered: in particular, touching only an inner group still commits to the outer one, whose own required arguments are then demanded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two findings from review of the defaulting commit. A defaulted field takes its default from a static member named `Default` plus the field name, so an awkward field name makes an awkward *member* name: a field named ``space in name`` wants `Owner.``Defaultspace in name`` ()`, and all three sites which emit that call wrote it bare, producing a file which does not parse. This was already true of a defaulted leaf and of the help text, which renders a leaf's default by calling the function at runtime; the defaulted-group commit added a third site with the same flaw. `backtickRecordLabel` is generalised to `backtickIdent` and applied at all three: the record-label probe is the right question for a member name too, being the stricter position. Separately, a defaulted group accepted a repeated [<ArgumentDefaultFunction>] where a defaulted leaf rejects it. The other two default-supplying attributes are already refused outright on a group, so repeating this one was the only way through; it now gives the same message the leaf path does. The `AwkwardDefaultName` fixture covers all three emission sites, and reverting any of them breaks the ConsumePlugin build, which is exactly the failure mode being fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 field whose type is another argument record, or a union of alternative argument sets, may now be wrapped in
optionorChoice<'a, 'a>: a whole group of arguments which need not be supplied.The group is present exactly when at least one argument beneath it was supplied — the same rule which chooses a union's case — and its own required arguments are then enforced as usual, so
--subject=hialone is an error demanding--emailrather than a quietly absent group.Previously a record or union was recognised only as a field's exact outer type, so one wrapper layer sent the field to the leaf machinery, which cannot parse a structural type and gave up with "Could not decide how to parse arguments".
How it works
One new generator-only
ParseTree.Containernode, which erases to a two-caseSumwhose second case is the empty product. No runtime change at all: "which case was selected" already means "was anything beneath supplied".It is a node of its own rather than a
Sumbecause it is not an alternation the author wrote, and help text must not present it as one — it prints the ordinary group header with(optional), and a union nested beneath keeps its own "exactly one of" framing.A payload which is itself satisfiable with no arguments is refused. Nothing the user could type would distinguish "the group was supplied, and everything in it took its default" from "the group was never mentioned", and for
Choicethe two can disagree in value, so this is a real modelling question the generated parser should not answer by silently preferring one reading.That gate is also what makes the erasure sound: the empty case can never be touched, and the payload is never empty-satisfiable, so exactly one case is always selectable and none of the runtime's three selection errors can arise. The synthetic case names are therefore unreachable in any message.
Deliberately out of scope
SomeArgs list(a repeated group),Choice<'a list, 'a list>andChoice<Map, Map>. The first has no defensible delimitation rule on a flat command line; the other two turned out confusing enough to want separate thought. A design note covering all three, and theAccumulationflattening they would need, is written up separately.Along the way
Two bugs, both pre-existing and both independently valid:
'a list list/'a option listproduced an assertion phrased as an internal error ("WoofWare.Myriad invariant violated") despite being reachable from ordinary source. The list arm ofcreateParseFunctionchecked only for maps, while every sibling arm rejects each nesting it cannot express; the[<PositionalArgs>]path had always rejected them properly, so both paths now give the same message.Default-prefixed member name was emitted bare. A field namedspace in nametakes its default fromDefaultspace in name, so the generated file did not parse. Three sites emit that call — a leaf's applied default, the help text, and (new here) a defaulted group's instantiation.Testing
The reference oracle could not previously generate a
Sumcase with no leaves at all —chooseCutsgives every case at least one named id — so the exact shapeContainerintroduces was untested despite being representable. Three properties now cover it, including one which confirms the gate is necessary rather than merely conservative: without it, the empty command line genuinely is ambiguous.Each commit was reviewed individually against its parent.
🤖 Generated with Claude Code