Implement the ! type

[canndrew]

This implements the never type (!) and hides it behind the feature gate #[feature(never_type)]. With the feature gate off, things should build as normal (although some error messages may be different). With the gate on, ! is usable as a type and diverging type variables (ie. types that are unconstrained by anything in the code) will default to ! instead of ().

(tracking issue #35121 )

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[canndrew]

How much effort is it worth going to to get this working without MIR? It works currently except for one weirdly failing test. But if we're going to be launching MIR soon is it worth keeping old trans working at all?

[eddyb]

This can be a bit more formal, e.g. "Remove this in the next snapshot." and not_stage0 instead of args.
cc @alexcrichton Does "SNAP" work anymore?

[eddyb]

@nikomatsakis What do you think about these impls?

[canndrew]

I'm pretty sure I added these out of necessity to make a test pass. Also ! is trivially totally ordered so it should impl Ord.

[bors]

☔ The latest upstream changes (presumably #35174) made this pull request unmergeable. Please resolve the merge conflicts.

[nikomatsakis]

I think that this function should be renamed -- it sounds very much like it is checking for TyEmpty, when in fact it does more (not sure if this happens in a later commit)

[nikomatsakis]

OK so @eddyb and I chatted a bunch on IRC. In general, I'm feeling nervous about how the "never-to-any" adjustment is different from other adjustments. For one thing, it's applied eagerly, in write_ty, and we want it to be reflected in expr_ty (which until now was basically an alias for node_ty). This also seems to suggest that some of the other logic for applying adjustments may be wrong -- although it seems like we already in some cases apply coercions even when another coerce exists, and the code is handling that case.

I can see various ways to go forward:

• Just merge it. Typeck is messy, gets a bit messier, up the priority on some kind of cleanup rewriting. I feel nervous about this, but it'd probably be ok.
• Rename expr_ty to expr_ty_adjusted and have it apply all adjustments. Rewrite uses of expr_ty to use that. This...would probably be more-or-less the same as now, because in general the adjustments haven't been applied (I think?) at the places where expr_ty is called (or, if they have, maybe we actually want to be seeing them?).
• Rewrite to return a Ty<'tcx> out of the "check expr" routine and instead of calling expr_ty everywhere, actually use the (adjusted) return value.

Honestly without trying to work through either of those latter two options, I'm not quite sure how easy/hard they would be, but it seems like option 3 probably leaves us with a cleaner setup than we started with overall (presuming it works), so maybe worth a try.

[bors]

☔ The latest upstream changes (presumably #35116) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

💔 Test failed - auto-linux-64-opt-no-mir

[arielb1]

running 2490 tests
Trace/breakpoint trap (core dumped)
make: *** [tmp/check-stage2-T-x86_64-unknown-linux-gnu-H-x86_64-unknown-linux-gnu-rpass.ok] Error 133

@bors retry
?

[eddyb]

@bors r-

@arielb1 This is most likely indicative of old trans being broken by this PR.

[arielb1]

Old trans apparently sometimes drops rvalue datums from other branches. You should use an lvalue datum instead.

fix:

diff --git a/src/librustc_trans/expr.rs b/src/librustc_trans/expr.rs
index 0ea5715..18bafdb 100644
--- a/src/librustc_trans/expr.rs
+++ b/src/librustc_trans/expr.rs
@@ -385,7 +385,7 @@ fnapply_adjustments<'blk,'tcx>(bcx:Block<'blk,'tcx>,
let mono_target = bcx.monomorphize(target);let llty = type_of::type_of(bcx.ccx(),mono_target);let dummy = C_undef(llty.ptr_to());
- datum = Datum::new(dummy, mono_target,Rvalue::new(ByRef)).to_expr_datum();
+ datum = Datum::new(dummy, mono_target,Lvalue::new("never")).to_expr_datum();}AdjustReifyFnPointer => {match datum.ty.sty{

test case (sans assertions):

pubstructReceiver(u32);implDropforReceiver{fndrop(&mutself){}}pubfnrecv(f1:bool,f2:bool) -> Receiver{match f1 {false => Receiver(0),true => {match f2 {true => returnReceiver(0),false => returnReceiver(0),}}}}fnmain(){}

[eddyb]

@bors r=nikomatsakis

[bors]

📌 Commit f59f1f0 has been approved by nikomatsakis

[canndrew]

@eddyb Thanks!

[canndrew]

Also @arielb1 thanks!

[bors]

⌛ Testing commit f59f1f0 with merge 1f58506...

[bors]

💔 Test failed - auto-win-msvc-64-opt-no-mir

[arielb1]

@bors retry

[canndrew]

Anyone know what happened here? Has the problem with msvc wanting libm been fixed?

[retep998]

I really doubt msvc wants libm. If that legitimately happened I'd be quite worried. It is probably just some other bug causing a random "m" to appear in the command to the linker.

[bors]

⌛ Testing commit f59f1f0 with merge e25542c...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-no-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-no-mir, auto-win-msvc-64-opt-rustbuild
Approved by: nikomatsakis
Pushing e25542c to master...

[durka]

For the record this indirectly introduced some issues (see #35883 (comment)) and parts of it will need to be semi-reverted (old Diverging branches replaced with new .is_never() branches).