feat: implement a simple hover auto traits in rpit - #23240
Conversation
Example
---
```rust
//- minicore: send, unpin
fn foo() -> $0impl Trait {
&raw const ()
}
```
->
> ```rust
> impl Trait + Unpin
> impl Trait = *const ()
> ```
>
> ---
>
> ```rust
> impl
> ```
>
> ---
>
> keyword docs
rustbot
added
the
S-waiting-on-review
| if traits.is_empty() { None } else { Some(traits.into_iter().collect()) } | ||
| } | ||
|
|
||
| #[salsa::tracked(returns(as_deref))] |
There was a problem hiding this comment.
Should IMO be the same query with crate_notable_traits(). Also both should check the presence of the unsafe feature like we do for crate_lang_items().
| Some(HoverResult { markup, actions }) | ||
| } | ||
|
|
||
| pub(super) fn rpit( |
There was a problem hiding this comment.
IMO this is the wrong way to fix. We should instead do that in display infra, for every printed opaque.
Granted, it'll need more work because we don't currently carry the Option<GenericDefId> required for trait solving (to get trait_environment()) in display.
There was a problem hiding this comment.
IMO this is the wrong way to fix. We should instead do that in display infra, for every printed opaque.
I think these are two different features, regarding RPIT and any expressions
fn foo() -> Cell<impl Trait> { Cell::new(2) }
//^^^^ impl Trait + Send + Sync + ...
// impl Trait = i32fn foo() -> Cell<impl Trait> { Cell::new(2) }
//^^^^^^^^^^^^ Cell<i32>
// implement auto traits: Send + ...fn foo() -> Cell<impl Trait> { Cell::new(2) }
//^ i32
// implement auto traits: Send + Sync + ...There was a problem hiding this comment.
No. I'm not saying to add a clause "implements auto traits" similar to "implements notable traits", only to add this information when displaying opaques.
There was a problem hiding this comment.
I think this is a feature that is bound to the function return, and the opaque semantics in other places are different, such as in the impl Trait of the function parameter
There was a problem hiding this comment.
This is a feature that is relevant to any opqaue - RPIT, ATPIT, ....
There was a problem hiding this comment.
Hmmm…, On the other hand, the current implementation is simple and very practical (in the stable version)
|
I feel like we should also generally render these for function hovers? Given auto trait leakage is here to stay showing them in all relevant places where we render the rpit feels appropriate, or at least for Send and Sync (more unconditionally is probably too much) |
I hope it's an expression. The last time the functions displayed 'notable traits', it caused me a lot of pain
These places should render 'auto traits' instead of 'rpit' For expressions, they should be simple, but for types (especially generics), I don't think we're ready yet |
|
I argue we should only show |
|
I think we should show all, but we can do some heuristic sorting or hard coding to put more useful results at the forefront |
|
What other auto traits you consider interesting?
|
|
I sometimes notice |
|
Given auto traits are perma-unstable feature, I absolutely think this should be a whitelist. |
|
Considering unstable rust users, won't they feel confused about r-a when using unstable features? |
|
Auto traits are perma-unstable. They will not be stabilized. If someone is using them, that's on them. |
|
Maybe, when someone complains, consider whether to improve or not
Although |
4 participants
Partial implement of #23237
Example
->