rust-lang / rust

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

Tracking issue for promoting `!` to a type (RFC 1216) #35121

Open nikomatsakis opened 8 years ago

nikomatsakis commented 8 years ago

Tracking issue for rust-lang/rfcs#1216, which promotes ! to a type.

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Pending issues to resolve

Interesting events and links

nagisa commented 7 years ago

Replacement for uninitialised is union { !, T }. You can get unreachable by ptr::read::<*const !>()ing and many other similar ways.

On Aug 8, 2017 3:58 PM, "Martin Habovštiak" notifications@github.com wrote:

@eddyb https://github.com/eddyb Ah, yeah, forgot that asm!() is unstable, so std::intrinsics::unreachable() might be used as well.

@tbu- https://github.com/tbu- Sure, I highly prefer that one instead of creating uninhabited type. The issue is, it's unstable, so if it was somehow impossible to unsafely mark branch of code as unreachable, it'd not only break existing code, but also make it unfixable. I think such thing would severely harm Rust reputation (especially considering main developers proudly claiming it being stable). As much as I love Rust, such thing would make me reconsider it being useful language.

@Eroc33 https://github.com/eroc33 my understanding is that some people suggest doing it, but it's mere suggestion, not definite decision. And I stand against such suggestion because it'd break backward compatibility, forcing Rust to become 2.0. I don't think there is any harm supporting it. If people are concerned about bugs, lint would be more appropriate.

@canndrew https://github.com/canndrew Why do you think uninitialized will be deprecated? I can't believe such thing. I find it extremely useful, so unless there exists a very good replacement, I don't see any reason for doing so.

Finally, I'd like to reiterate that I agree that intrinsics::unreachable() is better than current hacks. That being said, I oppose forbidding or even deprecating those hacks until there is a sufficient replacement.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/35121#issuecomment-320948013, or mute the thread https://github.com/notifications/unsubscribe-auth/AApc0iEK3vInreO03Bt6L3EAByBHQCv9ks5sWFt3gaJpZM4JYi9D .

Kixunil commented 7 years ago

@nagisa Thanks! Seems like it'd solve the problem on technical level.

scottmcm commented 7 years ago

Would it be possible to carve out a subset of this so that ! could be used as a type parameter in return types? Right now if you have a stable function

fn foo() -> ! { ··· }

And want to use ?, you can't make the usual transformation to

fn foo() -> io::Result<!> { ··· }

Do the thorny coercion and type parameter default questions affect that case?

arielb1 commented 7 years ago

For matches, see https://public.etherpad-mozilla.org/p/rust-compiler-design-sprint-paris-2017-ucg

canndrew commented 7 years ago

40801 Can be ticked off.

arielb1 commented 6 years ago

We should try to fix #43061 before we stabilize !.

dtolnay commented 6 years ago

I didn't see any open I-unsound issues mentioning never_type so I filed a new one for this: #47563. Things seem to assume that [!; 0] is uninhabited but I can create one with a simple [].

nikomatsakis commented 6 years ago

@dtolnay added to issue header

nikomatsakis commented 6 years ago

@canndrew how are you for time? I'd like to see a subset of ! usage get stabilized (excluding the rules around what is exhaustive). I've kind of lost the thread of where we are, though, and I think it needs a champion. Do you have time to be that person?

canndrew commented 6 years ago

@nikomatsakis I'm sure I can find some time to work on this stuff. What needs to be done exactly though? Is it just the bugs linked on this issue?

canndrew commented 6 years ago

It looks like @varkor already has PRs open to fix the remaining issues (awesome!). As far as I can see, the only things left to do are to decide if we're happy with the traits that are currently implemented and to move/rename the feature gate so that it only covers the exhaustive pattern-matching changes.

canndrew commented 6 years ago

Although another thing: Do we want to make mem::uninitialized::<!>() panic at run-time (it currently causes UB)? Or should we leave those sorts of changes for now? I'm not up-to-speed with what's going on with the unsafe code guidelines stuff.

RalfJung commented 6 years ago

I think the plan is still https://github.com/rust-lang/rfcs/pull/1892, which I just noticed you wrote. :)

canndrew commented 6 years ago

@RalfJung Is there anything in particular blocking that? I could write a PR today which adds MaybeUninit and deprecates uninitialized.

SimonSapin commented 6 years ago

@canndrew Since many projects want to support all three release channels and uninitialized is available on stable, it’s preferable to only start emitting deprecation warnings on Nightly once the replacement is available on the stable channel. Soft deprecation through doc-comments is fine.

canndrew commented 6 years ago

I've created a PR for stabilizing !: https://github.com/rust-lang/rust/pull/47630. I dunno if we're ready to merge it yet. We should at least wait to see if @varkor's PRs fix the remaining open issues.

canndrew commented 6 years ago

I also think we should revisit https://github.com/rust-lang/rfcs/pull/1699 so that people can start writing elegant trait impls for !.

varkor commented 6 years ago

@canndrew: Although that RFC wasn't accepted, it looks remarkably similar to the proposal in https://github.com/rust-lang/rust/issues/20021.

canndrew commented 6 years ago

https://github.com/rust-lang/rust/issues/36479 should probably be added to the issue header aswell.

@varkor it is very similar. With your unreachable code work have you noticed any problems with code like this now being flagged as unreachable?:

impl fmt::Debug for ! {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        *self    // unreachable!
    }
}

Because that would pretty much necessitate accepting some kind of proposal like those.

nikomatsakis commented 6 years ago

@canndrew something I've been advocating lately -- though we've yet to develop a formal template -- is a kind of "summary issue" that attempts to clearly delineate what we are stabilizing. You can think of it as a report after the RFC, trying to summarize in sort of a bullet-list form the salient features of what is being stabilize but also what is not.

Part of this would be pointers to test cases that demonstrate the behavior in question.

Do you think you could try to draw up such a thing? We can chat a bit together if you like about a kind of "outline" -- or I can maybe try to sketch it out.

nikomatsakis commented 6 years ago

Related question: should we try to resolve https://github.com/rust-lang/rust/issues/46325 before we stabilize? Maybe it doesn't matter.

earthengine commented 6 years ago

@nikomatsakis I vote for not to wait for any warning only issues to be resolved. That is harmless. If no further real concerns appeared, I think we should go ahead and stabilize it.

varkor commented 6 years ago

@canndrew: I don't think I've really looked at any cases like that, though I definitely think being able to omit impossible implementations will be much-needed when ! is stabilised.

canndrew commented 6 years ago

@nikomatsakis

Do you think you could try to draw up such a thing? We can chat a bit together if you like about a kind of "outline" -- or I can maybe try to sketch it out.

I could write up a draft at least and you could tell me if it's what you had in mind. I'll try to get it done over the next couple of days.

canndrew commented 6 years ago

@nikomatsakis Something like this?

Summary issue - stabilization of !

What is being stabilized

What is not being stabilized

canndrew commented 6 years ago

should we try to resolve #46325 before we stabilize?

As nice as it would be to clean up every loose end like this, it doesn't really seem like a blocker.

nikomatsakis commented 6 years ago

@canndrew

Something like this?

Yep, thanks! That's great.

The main thing that is missing are pointers to test cases showing how ! behaves. The audience should be the lang team or other people following closely, I would think, it's not really targeting "general purpose" folks. So e.g. I'd like some examples of places where coercions are legal, or where ! can be used. I'd also like to see the testes that show us that exhaustive pattern matching (without feature gate enabled) still behaves. These should be pointers into the repository.

nikomatsakis commented 6 years ago

@canndrew

As nice as it would be to clean up every loose end like this, it doesn't really seem like a blocker.

Well, it means that we'll enable new code that is going to immediately change behavior (eg., let x: ! = ... will I think behave differently for some expressions). Seems best to resolve if we can. Maybe you can make it a hard error on the PR you had open and we can lump it in with the existing crater run on the PR?

canndrew commented 6 years ago

@nikomatsakis I've updated that summary issue again with some links and examples. Also, sorry it's taken me a while to get to this, I've been very busy this past week.

canndrew commented 6 years ago

Maybe you can make it a hard error on the PR you had open and we can lump it in with the existing crater run on the PR?

Done.

nikomatsakis commented 6 years ago

@rfcbot fcp merge

I propose that we stabilize the ! type -- or least part of it, as described here. The PR is here.

One bit of data I would like is a crater run. I am in the process of rebasing https://github.com/rust-lang/rust/pull/47630 (since @canndrew isn't responding to pings just now) so we can get that data.

rfcbot commented 6 years ago

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

nikomatsakis commented 6 years ago

Oh, a couple of things I just remembered:

SimonSapin commented 6 years ago

@nikomatsakis Could we change the fallback rule in a new epoch but still make ! as a type available in the 2015 epoch?

canndrew commented 6 years ago

@nikomatsakis

We should consider the idea of stabilizing this only in the new epoch.

Last time we did a crater run (which was a long time ago) the fallout from changing the fallback rules was fairly minimal. We've also been linting against code that could be effected by the change for a while.

Second, I believe that part of the plan here was also to have some guidelines for when it is appropriate to implement a trait for !.

This is mentioned in the docs for !

nikomatsakis commented 6 years ago

@SimonSapin

Could we change the fallback rule in a new epoch but still make ! as a type available in the 2015 epoch?

Yes

nikomatsakis commented 6 years ago

@canndrew

Last time we did a crater run (which was a long time ago) the fallout from changing the fallback rules was fairly minimal. We've also been linting against code that could be effected by the change for a while.

Yep. Let's see what crater says. But, as I said, crater only ever gives us a lower bound -- and this is kind of an "elective change". Still, I suspect you are right and we can "get away" with changing this without much effect on code in the wild.

arielb1 commented 6 years ago

The TL;DR is that it is ok if the methods in the trait are unusable without first supplying a ! value

That's just the normal rule - you add an impl when you can implement it in a sane way, where "implement it in a sane way" excludes panicking, but includes "ex falso" UB in the presence of invalid data.

glaebhoerl commented 6 years ago

@arielb1 Yes, but for some reason people tend to get confused about things like this in the presence of !, so it seems worth calling out explicitly.

RalfJung commented 6 years ago

Maybe it'd help to have a safe method fn absurd(x: !) -> ! that's documented to be a safe way to express unreachable code or so somewhere in libstd? I think there was an RFC for that... or at least an RFC issue: https://github.com/rust-lang/rfcs/issues/1385

SimonSapin commented 6 years ago

@RalfJung Isn’t that absurd function just identity? Why is it useful?

It’s not the same as the unsafe fn unreachable() -> ! intrinsic which doesn’t take any argument and is very undefined-behaviory.

RalfJung commented 6 years ago

Well, yeah, it mostly is. I was using the ! return type in the sense of "does not terminate". (The usual type of absurd is fn absurd<T>(x: !) -> T, but that doesn't seem useful in Rust either.)

I was thinking maybe this would help with "people tend to get confused about things like this in the presence of !" -- having some documentation somewhere that "ex falso" reasoning is a thing.

Seems I also got the wrong issue... I thought somewhere there was a discussion about such an "ex falso" function. Seems like I'm wrong.

SimonSapin commented 6 years ago

fn absurd<T>(x: !) -> T can also be written match x {}, but that’s hard to come up with if you haven’t seen it before. It’s at least worth pointing out in rustdoc and in the book.

RalfJung commented 6 years ago

fn absurd(x: !) -> T can also be written match x {}, but that’s hard to come up with if you haven’t seen it before.

Right, that's why I suggested a method somewhere in libstd. Not sure where the best place is though.

Ralith commented 6 years ago

The nice thing about absurd is that you can pass it to higher-order functions. I'm not sure this is ever necessary given how ! behaves wrt. unification, though.

eddyb commented 6 years ago

fn absurd<T>(x: !) -> T can also be written match x {}

It can also be written as just x (AFAIK) - you only need match if you have an empty enum.

canndrew commented 6 years ago

An absurd(x: !) -> T function would be useful for passing to higher-order functions. I've needed this (for instance) when chaining futures, one of which has error type ! and the other error type T. Even though an expression of type ! can coerce into T that doesn't mean that ! and T will unify, so sometimes you still need to manually convert the type.

Similarly, I think Result-like types should have an .infallible() method which converts Result<T, !> to Result<T, E>.

RalfJung commented 6 years ago

Result-like types should have an .infallible() method which converts Result<T, !> to Result<T, E>.

I would have expected such a method to have type Result<T, !> -> T.

Amanieu commented 6 years ago

I would have expected such a method to have type Result<T, !> -> T.

Isn't this just the same as unwrap? The panic will be optimized away since the Err case is unreachable.

Ericson2314 commented 6 years ago

@Amanieu The point is that there's no panic to begin with. I'd love to have a panic lint, and that would get tripped up if we relied on optimization. Even without the lint, using unwrap adds a footgun during refactors, less the panic become live code once again.