Slight Tweaks to Slice Compiler Definitions#769
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Slice compiler definition schema to align terminology (“options”) with icerpc-csharp and to place anonymous types in the canonical order (Result, Sequence, Dictionary) across the Rust hand-mapped definitions and the Slice IDL.
Changes:
- Renamed
args: Argumentstooptions: Optionsin theCodeGeneratorinterface and renamed the corresponding typealias. - Reordered anonymous type definitions so
ResultTypeappears beforeSequenceTypeandDictionaryType. - Reordered the
Symbolenum variants to the canonical order (including updating Rust explicit discriminant values).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
slicec/src/definition_types.rs |
Reorders ResultType and updates Symbol variant ordering/IDs to match canonical anonymous type ordering. |
slice/Compiler/SyntaxElements.slice |
Reorders anonymous type struct declarations and updates doc wording to reflect canonical order. |
slice/Compiler/CodeGenerator.slice |
Renames generator “args” to “options” and reorders Symbol variants accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[repr(u8)] | ||
| #[derive(Clone, Debug)] | ||
| pub enum Symbol { | ||
| Interface(Interface) = 0, |
There was a problem hiding this comment.
What is the logic for this sorting?
There was a problem hiding this comment.
I think it's loosely based on "similarity" and "language importance".
So, all the anonymous types should be next to one another, since they're conceptually similar. Same for all the user defined types. Typealias goes at the bottom because it's not similar to anything else, and is also the least important, and least useful of Slice constructs.
Then within each grouping, it's kind of just what feels right I guess. Sequence feels like it should come before Dictionary. Result should come before either of them because it has more in common with the user-defined types than it has with the collection types.
But this PR was just to make them consistent with the order we settled on them for slicec. If you have any other elements you want to swap around, feel free to comment!
Custom could be closer to TypeAlias, Enum could move beneath Struct, etc.
This PR renames 'arguments' to 'options' to match the terminology used by
icerpc-csharp.It also re-orders the anonymous types so that they are in the canonical order we always use:
Result,Sequence,Dictionary