Implement the `!` type #35162

canndrew · 2016-08-01T12:09:01Z

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 · 2016-08-01T12:09:16Z

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 · 2016-08-01T12:15:45Z

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 · 2016-08-01T12:28:30Z

src/libcore/cmp.rs

@@ -699,6 +699,39 @@ mod impls {

    ord_impl! { char usize u8 u16 u32 u64 isize i8 i16 i32 i64 }

+    // Note: This macro is a temporary hack that can be remove once we are building with a compiler
+    // that supports `!`
+    macro_rules! argh {


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 · 2016-08-02T20:01:47Z

src/libcore/cmp.rs

+    }
+
+    #[cfg(not(stage0))]
+    not_stage0!();


@nikomatsakis What do you think about these impls?

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 · 2016-08-03T17:55:47Z

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

nikomatsakis · 2016-08-03T20:47:51Z

src/librustc/ty/sty.rs

@@ -930,11 +933,15 @@ impl<'a, 'gcx, 'tcx> TyS<'tcx> {
        }
    }

-    pub fn is_empty(&self, _cx: TyCtxt) -> bool {
+    pub fn is_empty(&self, cx: TyCtxt) -> bool {


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 · 2016-08-03T23:10:32Z

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 · 2016-08-06T05:15:33Z

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

bors · 2016-08-14T06:22:45Z

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

arielb1 · 2016-08-14T09:10:45Z

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 · 2016-08-14T16:13:48Z

@bors r-

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

arielb1 · 2016-08-14T22:58:29Z

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 @@ fn apply_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):

pub struct Receiver(u32);
impl Drop for Receiver {
    fn drop(&mut self) {}
}

pub fn recv(f1: bool, f2: bool) -> Receiver {
    match f1 {
        false => Receiver(0),
        true => {
            match f2 {
                true => return Receiver(0),
                false => return Receiver(0),
            }
        }
    }
}

fn main() {}

eddyb · 2016-08-15T05:46:26Z

@bors r=nikomatsakis

bors · 2016-08-15T05:46:27Z

📌 Commit f59f1f0 has been approved by nikomatsakis

canndrew · 2016-08-15T05:47:58Z

@eddyb Thanks!

canndrew · 2016-08-15T05:49:43Z

Also @arielb1 thanks!

bors · 2016-08-15T10:05:18Z

⌛ Testing commit f59f1f0 with merge 1f58506...

bors · 2016-08-15T12:12:34Z

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

arielb1 · 2016-08-15T22:01:01Z

@bors retry

canndrew · 2016-08-16T05:00:01Z

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

retep998 · 2016-08-16T05:09:18Z

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 · 2016-08-16T07:12:13Z

⌛ Testing commit f59f1f0 with merge e25542c...

Implement the `!` type 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 `()`.

bors · 2016-08-16T11:41:26Z

durka · 2016-08-22T16:56:40Z

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).

rust-highfive assigned pnkfelix Aug 1, 2016

eddyb reviewed Aug 1, 2016
View reviewed changes

canndrew force-pushed the bang_type_coerced branch 2 times, most recently from f57d48f to 143ba6e Compare August 2, 2016 18:35

eddyb reviewed Aug 2, 2016
View reviewed changes

canndrew force-pushed the bang_type_coerced branch from f9c44b3 to 54b7243 Compare August 3, 2016 07:05

nikomatsakis self-assigned this Aug 3, 2016

canndrew force-pushed the bang_type_coerced branch from 54b7243 to 2c62c30 Compare August 3, 2016 18:07

nikomatsakis reviewed Aug 3, 2016
View reviewed changes

Fix bug for ! in old trans

f59f1f0

bors merged commit f59f1f0 into rust-lang:master Aug 16, 2016

This was referenced Aug 16, 2016

Remove the ParamSpace separation from formal and actual generics in rustc. #35605

Merged

[MIR] Dataflow framework, constant propagation and dead code elimination #35608

Closed

arielb1 added the relnotes Marks issues that should be documented in the release notes of the next release. label Aug 16, 2016

dtolnay mentioned this pull request Aug 17, 2016

Does not compile on 1.12.0-nightly (1bf5fa326 2016-08-16) serde-deprecated/aster#100

Closed

lambda-fairy mentioned this pull request Aug 18, 2016

Make Void an alias for ! reem/rust-void#13

Open

This was referenced Aug 22, 2016

typeck: use NoExpectation to check return type of diverging fn #35883

Merged

no explicit returns in diverging fns, even if the returned expr diverges #35888

Closed

Sh4rK mentioned this pull request Aug 26, 2016

non-exhaustive patterns: type ! is non-empty #36011

Closed

SimonSapin mentioned this pull request Dec 20, 2018

Stabilize never_type *again* #57012

Closed

SimonSapin mentioned this pull request Feb 25, 2019

The never type can be named despite its feature gate #58733

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the `!` type #35162

Implement the `!` type #35162

canndrew commented Aug 1, 2016 •

edited by pnkfelix

Loading

rust-highfive commented Aug 1, 2016

canndrew commented Aug 1, 2016

eddyb Aug 1, 2016

eddyb Aug 2, 2016

canndrew Aug 3, 2016

bors commented Aug 3, 2016

nikomatsakis Aug 3, 2016

nikomatsakis commented Aug 3, 2016 •

edited

Loading

bors commented Aug 6, 2016

bors commented Aug 14, 2016

arielb1 commented Aug 14, 2016

eddyb commented Aug 14, 2016

arielb1 commented Aug 14, 2016

eddyb commented Aug 15, 2016

bors commented Aug 15, 2016

canndrew commented Aug 15, 2016

canndrew commented Aug 15, 2016

bors commented Aug 15, 2016

bors commented Aug 15, 2016

arielb1 commented Aug 15, 2016

canndrew commented Aug 16, 2016

retep998 commented Aug 16, 2016

bors commented Aug 16, 2016

bors commented Aug 16, 2016

durka commented Aug 22, 2016

Implement the ! type #35162

Implement the ! type #35162

Conversation

canndrew commented Aug 1, 2016 • edited by pnkfelix Loading

rust-highfive commented Aug 1, 2016

canndrew commented Aug 1, 2016

eddyb Aug 1, 2016

Choose a reason for hiding this comment

eddyb Aug 2, 2016

Choose a reason for hiding this comment

canndrew Aug 3, 2016

Choose a reason for hiding this comment

bors commented Aug 3, 2016

nikomatsakis Aug 3, 2016

Choose a reason for hiding this comment

nikomatsakis commented Aug 3, 2016 • edited Loading

bors commented Aug 6, 2016

bors commented Aug 14, 2016

arielb1 commented Aug 14, 2016

eddyb commented Aug 14, 2016

arielb1 commented Aug 14, 2016

eddyb commented Aug 15, 2016

bors commented Aug 15, 2016

canndrew commented Aug 15, 2016

canndrew commented Aug 15, 2016

bors commented Aug 15, 2016

bors commented Aug 15, 2016

arielb1 commented Aug 15, 2016

canndrew commented Aug 16, 2016

retep998 commented Aug 16, 2016

bors commented Aug 16, 2016

bors commented Aug 16, 2016

durka commented Aug 22, 2016

Implement the `!` type #35162

Implement the `!` type #35162

canndrew commented Aug 1, 2016 •

edited by pnkfelix

Loading

nikomatsakis commented Aug 3, 2016 •

edited

Loading