-
Notifications
You must be signed in to change notification settings - Fork 816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix error with complex enums with many fields #4832
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super, many thanks! Happy New Year :)
let spec = FnSpec::parse( | ||
&mut match_args_impl.sig, | ||
&mut match_args_impl.attrs, | ||
Default::default(), | ||
)?; | ||
let variant_match_args = impl_py_class_attribute(variant_cls_type, &spec, ctx)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had an initial feeling that building the syntax tree and then parsing it might be a bit convoluted (we could just construct a FnSpec
directly) but maybe this is actually quite elegant because it means the #[classattr]
defined above fully defines the final result.
It would have been nice to avoid the fallibility, but I think it's fine on balance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it means the
#[classattr]
defined above fully defines the final result.
This was my motivation for this. I think it's more robust against accidental inconsistencies (now and in the future)
It would have been nice to avoid the fallibility, but I think it's fine on balance.
Yeah, I was a bit sad as well . It should never fail, so we could also expect
, but that did not feel much better to me.
Happy New Year to you as well 🎆
Changes the expansion of
__match_args__
for complex enums to not use Rust tuples to lift the limit of 12 fields.Closes #4827