search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.36k stars 12.59k forks source link

Tracking issue for RFC 2397, "#[do_not_recommend]" #51992

Open Centril opened 6 years ago

Centril commented 6 years ago

This is a tracking issue for the RFC "Introduce #[do_not_recommend] to control errors for trait impls" (rust-lang/rfcs#2397).

Steps:

Unresolved questions:

implementation history:

nagisa commented 6 years ago

Minor question. Presuming we implement this, and later change diagnostics in ways that make this attribute unnecessary/harmful, what would the plan of action be then?

Does this attribute essentially encode some particular format of these specific errors?

Centril commented 6 years ago

@nagisa in that hypothetical situation, deprecate the attribute perhaps?

scottmcm commented 2 years ago

This came up in T-lang's backlog bonanza today. With no progress on this for years, it's not clear to us that this is still the right way forward here.

@estebank, do you have any thoughts on this one? Perhaps it would be better rolled into the idea you brought up in https://internals.rust-lang.org/t/stable-diagnostic-affecting-attribute-with-unstable-api/16368?u=scottmcm?

c410-f3r commented 1 year ago

@rustbot claim

estebank commented 1 year ago

If the proposal for #[diagnostics:*] is indeed accepted (which I'm confident now it will be), I strongly believe this should live under that namespace.

weiznich commented 1 year ago

@estebank It should be fine to implement this as unstable feature without waiting for the #[diagnostics] RFC to be accepted. I would guess that the attribute can be moved afterwards easily, as long as it is not part of any stable release yet?

estebank commented 1 year ago

@weiznich that's my understanding too.

alice-i-cecile commented 7 months ago

Now that https://github.com/rust-lang/rust/pull/119888 is merged, what's the next step forward for this?

c410-f3r commented 7 months ago

This feature had to be temporarily paused due to concerns around possible refactors and the new solver. https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.E2.9C.94.20RFC-2397

If @lcnr thinks that the new solver is in a good state, then I can resume my activities with or without the diagnostic namespace.

lcnr commented 6 months ago

Sorry for taking so long to reply to this issue.

The solver is not yet in a good enough state to support work on that attribute. However, looking at the old solver again, we already track where nested obligations come from: https://github.com/rust-lang/rust/blob/8d490e33ad7bbcdeab7975be787653b1e46e48e4/compiler/rustc_trait_selection/src/traits/select/mod.rs#L2755-L2783

This info could be used to implement it in the old solver without requiring any additional tracking: if the cause of an FUlfillmentError contains a impl_or_alias_def_id with the #[diagnostics::do_not_recommend] attribute, simply use the root obligation. This isn't perfect, as #[do_not_recommend] hides non-root parent goals we may actually have wanted to recommend.

It will take at least another few months until the "proof tree visitor" in the new solver is at a state where it can be used for diagnostics. Right now we still have to change that code to fix the happy path behavior and I don't want to slow down progress on this by having to migrate diagnostics code each time we do so.

c410-f3r commented 6 months ago

Thank you @lcnr