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

Support local trait impls for non-local types for unnamed free constants #7550

Closed
edwin0cheng opened this issue Feb 4, 2021 · 5 comments · Fixed by #8390
Closed

Support local trait impls for non-local types for unnamed free constants #7550

edwin0cheng opened this issue Feb 4, 2021 · 5 comments · Fixed by #8390
Labels
A-nameres name, path and module resolution A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now

Comments

@edwin0cheng
Copy link
Member

edwin0cheng commented Feb 4, 2021

In following code:

trait Foo {}
struct A {}

const _ : () = {
    impl crate::Foo for crate::A {}  <---  (1)
};

(1) is local trait impls for non-local types. Currently we do not support it due to the fact that we do not want to look at the bodies of all functions and blocks. But is it possible to support this case solely, as an escape hatch for proc-macro author to workaround of lacking of proc-macro hygiene at the moment ?

What do you think about it @jonas-schievink ?

@jonas-schievink
Copy link
Contributor

Yeah, I think this is a good idea!

@jonas-schievink
Copy link
Contributor

If there's any use items in the initializer, then this might need better support for inner items first.

Then we have to stop ignoring unnamed consts here and record them as part of the containing DefMap, like we do for other items:

https://github.com/rust-analyzer/rust-analyzer/blob/700a3d5d75cdc1b8b7049d35af335c638e079c5c/crates/hir_def/src/nameres/collector.rs#L986

Finally, we have to change the trait_impls_in_crate query to inspect the body of all anonymous consts to look for blocks containing impls (this might need further changes to body lowering first):

https://github.com/rust-analyzer/rust-analyzer/blob/6bde542a39fe63298a31b838e59705797ed8a2cf/crates/ra_hir_ty/src/method_resolution.rs#L49

It is possible that there's a more optimal way of doing this though, but this should be a start.

@edwin0cheng edwin0cheng added S-actionable Someone could pick this issue up and work on it right now E-has-instructions Issue has some instructions and pointers to code to get started labels Feb 4, 2021
@flodiebold
Copy link
Member

flodiebold commented Feb 4, 2021

Maybe we should even restrict it to unnamed consts of unit type? That would be annoying to check during name resolution though.

@jonas-schievink
Copy link
Contributor

We can check for a literal () type, ignoring type aliases and associated types. Not sure if that's worth it though, anonymous consts might be used rarely enough where it doesn't matter if we always do it.

@flodiebold
Copy link
Member

Actually, thinking about it, I can't think of any reason to make an anonymous const with a different type, so it probably wouldn't help anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-nameres name, path and module resolution A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants