Add #[with_unstable] to impls (or rustc_internals::marker::Stable)

1

Idea

The idea is that a new (internal/unstable) attribute, #[with_unstable], would exist and be applicable to impls. As in,

#[with_unstable]
impl Foo for Bar {
}

This is different from marking impls as #[unstable] in that it changes how impls are applied rather than whether you're allowed to use an impl. In particular:

  1. If Bar references an unstable type, it should be a compile error unless #[unstable] is used.
  2. If Bar is generic, unstable types shouldn't resolve to said generics unless #[unstable] is used.

On stable, this attribute would be completely ignored, due to backwards compatibility.

Alternatively:

  1. Every type is rustc_internals::marker::Stable by default
  2. Generics are : rustc_internals::marker::Stable by default
  3. (1) can be opted out with #[unstable]
  4. (2) can be opted-out with : ?rustc_internals::marker::Stable (similar to ?Sized)

(Note: this is both stronger and weaker than #[with_unstable]: it applies to functions, but not to non-generic trait impls. In particular #[derive(Eq, Hash, ...)] on non-Stable types would still work. Nevertheless it should not affect e.g. std::str::pattern::Pattern in str methods because that's a trait bound rather than a type argument, and the impls exist on stable types.)

Motivation

Option to Result was a mistake:

You can't fix this on stable. However, you can break this on nightly. Since nightly doesn't have the stability guarantees it's technically perfectly allowed to break compatibility with broken "features" (aka mistakes) on nightly. To encourage users to stop converting Option to Result, one just has to break Option to Result on nightly and call it a day. Stable support wouldn't be affected so it's fine.

Benefits

Code like this shouldn't work in the first place, and the fact that it does is arguably a bug. This is just fixing that bug - on nightly. It is extremely discouraged to write code that converts Option to Result like that, and Rust users are pretty good about chastising anyone who attempts to do so, which is nice. But it's not quite good enough, and there should be more motivations for not doing it. Making sure said code doesn't compile on nightly (altho with many easy workarounds - just throw #[with_unstable] at it, or change the code to use .ok_or_else or whatever) would provide such motivation.

Drawbacks

This would cause nightly behaviour to diverge from stable behaviour. If this is actually undesired, even for cases like this, then Rust would have to mark NoneError as stable and deprecated. We believe this is the next step to getting rid of NoneError tho.

2

(Thanks for adding the extra detail to your post!)

Since we can never remove the unintentional behavior on stable, the main benefit of disabling it on nightly is just a nudge to encourage people to stop using it, right? What are the practical benefits of getting people to stop using this, if we can't actually remove support for it? What are the negative effects of someone writing an impl that applies to NoneError?

I can see that the resulting code is less clear than it could be, but that seems like a fairly minor effect that may not be worth a large effort to prevent. If we do nothing, are we significantly worse off in some way I'm not seeing?

Or if we must do something, could we achieve a similar benefit for less effort by adding a warn-by-default lint (triggered by use of ? on Option in a non-Option context)? Advantages of a lint include:

  • The "nudge" would apply to users of any Rust toolchain, not just nightly.
  • Does not introduce new differences between nightly and stable.
  • Can use existing attributes to opt in/out, instead of introducing a new one.
(previous reply)

When you propose new features/changes, could you please try to provide more background, so they are understandable to people who are not already familiar with your motivation?

Please include, at minimum:

  1. The problem you want to solve.
  2. Examples of code/projects affected by the problem.
  3. Links to relevant previous issues, discussions, or proposals.
  4. Some code examples comparing how things work with or without the proposed solution.
  5. Any drawbacks or alternatives to the proposed solution.

For example, I think this is a reference to this and this. If so, including these links would make your post much more understandable to anyone not already familiar with that particular problem.

I keep harping on this because lack of context tends to generate a lot of misunderstanding in these posts, as people are forced to guess at the underlying problem or motivation (and often guess wrong). This leads to unproductive discussions with people talking past each other instead of understanding each other.

Deliberately making nightly non-backward-compatible has some pretty serious drawbacks, enough that I would expect it to be a non-starter. If you want to keep making proposals that potentially break existing users, you need to acknowledge the downsides and also make a strong case that they are outweighed by the benefits.

7 Likes
3

We don't understand why it can't ever be removed tbh - it is effectively a bug, after all. And if it gets removed, the attribute can be made to work on stable, with the side-benefit that things like this would never happen again.

A deny-by-default/soft-deprecation would also work but doesn't prevent things like this from happening again in the future.

4

It doesn't cause unsoundness or other severe problems, so it's more of an undesired feature. Not everybody might agree that it's a problem at all. So there definitely has to be a warn-by-default lint before attempting to deprecate it. And deprecating it only on nightly seems like a half-hearted solution that has more drawbacks than advantages.

2 Likes
5

Deprecating it on nightly is more about (eventually) preventing things like this from ever happening again, than about it specifically.

6

Having unstable-to-use trait impls wouldn't have helped in ?'s option-to-result case, anyway, though.

The problem there was that a trait impl that provided desired stable behavior option? in -> Option<_> accidentally also stabilized option? in -> Result<_, _>. The two bits of functionality are (currently) one and the same, and it's not possible to have one without the other.

There definitely is interest in being able to make trait implementations unstable in the future. But it's just been low priority so far, and isn't going to be used to hard deprecate stable functionality.

7

These two functionalities will be separate once the try_trait_v2 RFC is accepted and implemented.

8

I'm aware, but my point is that whatever functionality @Soni requests wouldn't have prevented the accidental stabilization in the first place.

9

The “accidental stabilization” happened because it's possible to use the unstable NoneError type without naming it, via generic impls like:

// `T` can be `NoneError`.
impl<T: std::fmt::Debug> From<T> for Tricky { ... }

The proposal, as I understand it, is that unstable types would not be allowed as type parameters in such impls, so it would be impossible for user types to implement From<NoneError> in this way. If we’d had this feature on all channels before NoneError was implemented, then I think it would be impossible to to use ? to convert Option to Result in stable Rust.

(Though I'm still not convinced it's actually feasible to implement this without greatly complicating the trait system, nor that it's feasible to transition to a world where it's implemented and enabled in stable Rust, nor that it's worth doing even if feasible.)

That RFC explicitly mentions, however, that this functionality must continue to be supported for backward compatibility.

2 Likes
10

Yeah, I read the sentence

so that Soni wants to undo the stabilization on nightly.

@mbrubeck Good point!

1 Like
11

The proposal is basically about introducing an behind-the-scenes ?Stable auto-trait, similar to ?Sized. Except hidden from everywhere by using an attribute instead of an actual type.

Edit: Now the proposal is about an actual ?Stable auto-trait because that has a lot better semantics.

12

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.