fix(mgca): Allow specifying generic args (of enum) on enum itself in unit & tuple variant constructions in (direct) const args#155198
Conversation
|
HIR ty lowering was modified cc @fmease |
This comment has been minimized.
This comment has been minimized.
…unit & tuple variant constructions in (direct) const args
f0b92a1 to
28594d7
Compare
|
Fixed that @fmease |
| .prohibit_generic_args(leading_segments.iter(), GenericsArgsErrExtend::None); | ||
|
|
||
| // Check if this is the Enum::<...>::Variant form | ||
| let use_enum_segment = if path.segments.len() == 2 |
There was a problem hiding this comment.
The path should be allowed to have more than two segments. Right now, you're only allowing Enum::<...>::Variant but not e.g., very::long::path::to::Enum::<...>::Variant. So it should be >= 2 rather than == 2.
Once you implement that you also need to make sure to reject arguments on all segments before the enum segment since module::<...>::Enum::<...>::Variant should be forbidden.
| let use_enum_segment = if path.segments.len() == 2 | ||
| && path.segments[0].args.is_some() | ||
| && matches!(path.segments[0].res, Res::Def(DefKind::Enum, _)) |
There was a problem hiding this comment.
Since we have leading_segments in scope (which would be equal to very::long::path::to::Enum::<...> in the example I gave above), we could use it here plus slice patterns. So something akin to:
| let use_enum_segment = if path.segments.len() == 2 | |
| && path.segments[0].args.is_some() | |
| && matches!(path.segments[0].res, Res::Def(DefKind::Enum, _)) | |
| let use_enum_segment = if [.., second_to_last] = leading_segments | |
| && second_to_last.args.is_some() | |
| && let Res::Def(DefKind::Enum, _) = second_to_last.res |
| true | ||
| } else { | ||
| let _ = self.prohibit_generic_args( | ||
| leading_segments.iter(), | ||
| GenericsArgsErrExtend::None, | ||
| ); | ||
| false | ||
| }; |
There was a problem hiding this comment.
Instead of returning true / false, you could return the argument-bearing segment to be later passed as an argument to lower_generic_args_of_path_segment.
There was a problem hiding this comment.
this part of the code is not removed in favour of probe_generic_path_segments
| // Enum::<...>::Variant form - allow enum segment to have args, use them | ||
| // instead of variant segment's args (which are typically empty) | ||
| true |
There was a problem hiding this comment.
You need to make sure to reject arguments on the variant segment if the enum segment has arguments since Enum::<...>::Variant::<...> should be forbidden.
There was a problem hiding this comment.
Is there are way to use probe_generic_path_segments instead? Maybe it just needs to be generalized or split into more reusable parts? Under the current version of this PR, we now have three places that implement the enum variant argument logic.
Of course, if they can't be unified easily we shouldn't force it.
There was a problem hiding this comment.
Yeah, it actually fits in pretty well, i changed it to use probe_generic_path_segments in 298d470, lmk if the implementation is good
There was a problem hiding this comment.
wasn't sure of how to fit prohibit_generic_args with this tho
|
@rustbot ready |
…generics/mgca/generic-args-on-enum-variant-segments`
View all comments
Closes #154915
Basically, we check if there are three segments and if the first one is a
DefKind::Enum, and then use the correct segment forEnum::<...>::Variantsyntax for bothDefKind::Ctor(_, CtorKind::Const | CtorKind::Fn)PS: i'm not sure if the common logic from these two branches could be extracted
r? fmease