Skip to content
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

Reflect derive macro expansion causes syntax ambiguity with Higher-Kinded Types. #8759

Closed
Themayu opened this issue Jun 5, 2023 · 1 comment · Fixed by #8761
Closed

Reflect derive macro expansion causes syntax ambiguity with Higher-Kinded Types. #8759

Themayu opened this issue Jun 5, 2023 · 1 comment · Fixed by #8761
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior

Comments

@Themayu
Copy link
Contributor

Themayu commented Jun 5, 2023

Bevy version

0.10.1

System information

Rustc version: 1.69.0 (84c898d65 2023-04-16)
Cargo version: 1.69.0 (6e9a83356 2023-04-12)
Platform: amd64 Windows 11 22H2 (OS Build 22621.1702)

The bug report

Bit of an edge case that it might be useful to correct: During the expansion of the Reflect derive macro on a struct that uses a higher-kinded type directly as one of its field types, the macro expands to some rather ambiguous bounds:

#[derive(bevy::reflect::Reflect)]
pub struct SingleLineDisplayState {
    #[reflect(ignore)]
    transform: for<'a> fn(&'a str) -> &'a str,
}

fn get_type_registration<T: bevy::reflect::GetTypeRegistration>() {}

fn caller() {
    get_type_registration::<SingleLineDisplayState>(); // error occurs here
}

// auto-generated by derive(Reflect) on SingleLineDisplayState, shown here for example purposes
impl bevy::reflect::GetTypeRegistration for SingleLineDisplayState
where
    for<'a> fn(&'a str) -> &'a str: Any + Send + Sync,
{
    ...
}

What went wrong

Unfortunately, rustc does not interpret this as where (for<'a> fn(&'a str) -> &'a str): Any (a higher-kinded bound of for<'a> fn(&'a str) -> &'a str), as is intended, but instead where for<'a> (fn(&'a str) -> &'a str): Any (a for<'a>-bound predicate of the non-higher-kinded type fn(&'a str) -> &'a str). This then causes the compiler to be unable to prove that SingleLineDisplayState does in fact implement GetTypeRegistration, leading to a compilation failure with the rather unhelpful error message "error: higher-ranked lifetime error".

Additional information

The issue arises when attempting to pass SingleLineDisplayState to any generic parameter that expects its type to implement GetTypeRegistration, as seen in the example above.

This is a simple grammar problem, as merely wrapping the type in parentheses in the struct definition resolves the ambiguity and fixes the problem. Perhaps Reflect could be augmented to place parentheses around the trait bounds it outputs, to prevent this ambiguity?

@Themayu Themayu added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 5, 2023
@Themayu Themayu changed the title Reflect derive macro expansion causes syntax ambiguity with Higher-Kinded Lifetimes. Reflect derive macro expansion causes syntax ambiguity with Higher-Kinded Types. Jun 5, 2023
@Themayu
Copy link
Contributor Author

Themayu commented Jun 5, 2023

I should note that the issue can be resolved on the caller's end by doing anything that would resolve the ambiguity, even if the resolution is implicit - for example, wrapping the HKT in an identity macro that simply takes tokens and returns them straight back to the code unchanged will solve the issue, as will merely wrapping it in parentheses. However, it would be very beneficial if Bevy itself accounted for the issue, as this solution is rather obscure (and results in a spurious unused_parens lint if you wrap it in parentheses.)

@MrGVSV MrGVSV added A-Reflection Runtime information about types and removed S-Needs-Triage This issue needs to be labelled labels Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants