Fix a Panic on Enums/Interfaces that Inherited from Anonymous Types - #802
Conversation
| @@ -51,7 +51,10 @@ impl<T: Element + ?Sized> TypeRef<T> { | |||
| impl<T: Type + ?Sized> TypeRef<T> { | |||
| // This intentionally shadows the trait method of the same name on `Type`. | |||
| pub fn type_string(&self) -> String { | |||
There was a problem hiding this comment.
This is a tangential change.
It lets us print a reference before the patching phase has run.
i.e. now we can safely print a reference while the parser is running.
For unpatched references, we print the exact text found in the Slice file.
For patched references, the behavior is unchanged.
| .into_iter() | ||
| .map(|base| base.downcast::<Interface>().unwrap()) | ||
| .filter_map(|base| { | ||
| let interface_ref = base.downcast::<Interface>(); |
There was a problem hiding this comment.
Now we check the result of downcast::<Interface> instead of assuming it would succeed.
If it is_err() we report and error and discard the base. Otherwise it gets kept like before.
| ) -> OwnedPtr<Enum> { | ||
| let underlying = underlying_type.map(|type_ref| type_ref.downcast::<Primitive>().unwrap()); | ||
| let underlying = underlying_type.and_then(|type_ref| { | ||
| let primitive_ref = type_ref.downcast::<Primitive>(); |
There was a problem hiding this comment.
Same for enums, we check if the downcast succeeded instead of blindly calling unwrap (assuming it will always work).
There was a problem hiding this comment.
Pull request overview
This PR prevents slicec from panicking when a Slice file uses an anonymous type (Sequence/Dictionary/Result) as an enum underlying type or as an interface base type, and instead emits a proper diagnostic (per #798).
Changes:
- Update the Slice grammar construction for interfaces/enums to report a diagnostic instead of
unwrap()-panicking on invalid patched TypeRefs. - Improve
TypeRef::type_string()so it can stringify both patched and unpatched references (supporting better diagnostics). - Expand/parameterize enum/interface tests to cover anonymous-type cases (and optional anonymous types) that previously crashed.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| slicec/tests/interfaces/inheritance.rs | Parameterizes invalid interface-base tests to include primitives and anonymous built-ins. |
| slicec/tests/enums/mod.rs | Adds coverage for anonymous underlying types and splits optional-underlying cases into clearer test groups. |
| slicec/src/parsers/slice/grammar.rs | Replaces unwrap() downcasts with diagnostic-emitting fallbacks for interface bases and enum underlyings. |
| slicec/src/grammar/elements/type_ref.rs | Makes TypeRef::type_string() work for both patched and unpatched refs (avoids panics and improves messages). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
In a Slice file, if you try to use an anonymous type (a sequence, dictionary or result type) as:
enum Foo : Sequence<int8>), orinterface Foo : Sequence<int8>)It would cause the compiler to crash, instead of correctly reporting an error.
This was due to a bad assumption we were making about these during the parsing phase.
This PR fixes both parts of #798.
This PR doesn't have any notes, since:
A) Nobody in their right mind was ever doing this, and it's clearly insane on the face of it.
B) It was and is still disallowed, this just changes the 'rejection' from a
panicto a propererrorwe can report.