Reflect proc-macro causes compile errors when using custom Result type

[kyunghoon]

Bevy version

v0.5.0

Operating system & version

MacOS 11.4

What you did

used the Reflect proc-macro

What you expected to happen

compile successfully

What actually happened

compile error, conflicting Result types

Additional information

the derive_reflect process macro is not hygienic, i prefer to use my own specialized Result type which only has one generic parameter. this macro mistakenly picks it up. fix is pretty easy, i could make a PR if quested.

[mockersf]

Could you provide a code sample to reproduce the issue? If you identified the root cause, a PR would be nice 🙂

[ThrashAbaddon]

@kyunghoon Do you have a code example that maintainers can use to reproduce the issue?

[MrGVSV]

Sounds like this is due to the proc macro not specifying the fully qualified path to Result (i.e. ::core::result::Result). This seems like it would be a trivial fix and should probably be done for Option (::core::option::Option) as well.

[elbertronnie]

I would like to solve this issue. But being new to Rust and this codebase, I wanted to clarify a few things:

• I have to replace Result, Option and their respective variants which are inside quote! with its fully qualified name.
• Do I have to do this in only bevy_reflect_derive crate or all the proc macros across the codebase?
• Why were only Result and Option chosen? Is it because they are often specialized? Should I do the same for other members of core and std too (like Default or Vec)?

[MrGVSV]

I would like to solve this issue. But being new to Rust and this codebase, I wanted to clarify a few things:

Awesome! Glad to have you on board! Contributing to something like Bevy is a great way to learn Rust, honestly. Feel free to shoot a message on Discord (#engine-dev or #reflection-dev are good places to start) if you need help.

I'd recommend maybe getting some feedback there first. I imagine there might be people opposed to this change as it adds verbosity for something that people "generally shouldn't do" like shadow the name of a type in Rust's prelude.

I have to replace Result, Option and their respective variants which are inside quote! with its fully qualified name.

Yep! Anywhere we generate code, we should probably be using the fully-qualified path, which is most often in the quote! macro.

For example, you would change quote!(Some(123)) to quote!(::core::option::Option::Some(123)).

Do I have to do this in only bevy_reflect_derive crate or all the proc macros across the codebase?

Possibly. It might be better to break things up as messing with a bunch of crates generally requires a lot more review. Maybe just start with bevy_reflect_derive?

Why were only Result and Option chosen? Is it because they are often specialized? Should I do the same for other members of core and std too (like Default or Vec)?

They were chosen because they were the first ones to come to mind :)

But yes, that list is not exhaustive. Vec, Box, etc. could also be included in this.