Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unit #18752

Merged
merged 4 commits into from Nov 16, 2014
Merged

Remove unit #18752

merged 4 commits into from Nov 16, 2014

Conversation

ghost
Copy link

@ghost ghost commented Nov 7, 2014

Closes #18614.

@eddyb
Copy link
Member

eddyb commented Nov 8, 2014

👍 Been wanting to do this myself but I was afraid of the performance implications of going through interning for each call to mk_nil.

Does the hash optimize out, at least?
Last I checked, LLVM could const-eval FNV when applied to strings longer than 16 bytes (I didn't try to push it harder).

@@ -496,7 +496,7 @@ fn all_constructors(cx: &MatchCheckCtxt, left_ty: ty::t,
ty::ty_bool =>
[true, false].iter().map(|b| ConstantValue(const_bool(*b))).collect(),

ty::ty_nil =>
ty::ty_tup(ref tys) if tys.is_empty() =>
vec!(ConstantValue(const_nil)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't const_nil be removed as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it so that witnesses in non-exhaustive patterns would be more descriptive (i.e. () rather than _) but we can probably achieve that differently.

@pcwalton
Copy link
Contributor

pcwalton commented Nov 9, 2014

It still might be nice for pedagogical reasons to refer to unit as a kind of separate type in the documentation, since it's so common. (Internally though this is a great refactoring.)

@ghost
Copy link
Author

ghost commented Nov 10, 2014

@pcwalton Agreed, the manual still refers to () the way you described. But I made it truthful about () not being a literal any more.

@ghost ghost mentioned this pull request Nov 10, 2014
bors added a commit that referenced this pull request Nov 11, 2014
@Manishearth
Copy link
Member

error: unresolved name `ast::TyNil`.
src/libsyntax/parse/mod.rs:1040           node: ast::TyNil,

@nikomatsakis
Copy link
Contributor

💃

bors added a commit that referenced this pull request Nov 15, 2014
bors added a commit that referenced this pull request Nov 16, 2014
@bors bors closed this Nov 16, 2014
@bors bors merged commit c425ed2 into rust-lang:master Nov 16, 2014
@ghost ghost deleted the remove-unit branch November 16, 2014 18:13
milibopp added a commit to milibopp/gfx-rs that referenced this pull request Nov 17, 2014
The exact version is rustc 0.13.0-dev (0b7b4f075 2014-11-16 22:36:50 +0000) and
the relevant upstream issues are rust-lang/rust#18752 and rust-lang/rust#18976.

Fixes gfx-rs#439.
milibopp added a commit to milibopp/gfx-rs that referenced this pull request Nov 17, 2014
The exact version is rustc 0.13.0-dev (0b7b4f075 2014-11-16 22:36:50 +0000) and
the relevant upstream issues are rust-lang/rust#18752 and rust-lang/rust#18976.

Fixes gfx-rs#439.
japaric pushed a commit to bheisler/criterion.rs that referenced this pull request Nov 19, 2014
japaric pushed a commit to bheisler/criterion.rs that referenced this pull request Nov 19, 2014
Ms2ger added a commit to servo/servo that referenced this pull request Nov 24, 2014
Ms2ger added a commit to servo/servo that referenced this pull request Nov 26, 2014
Ms2ger added a commit to servo/servo that referenced this pull request Nov 28, 2014
Ms2ger added a commit to servo/servo that referenced this pull request Nov 29, 2014
Ms2ger added a commit to servo/servo that referenced this pull request Dec 2, 2014
GauravBholaris pushed a commit to GauravBholaris/criterion.rs that referenced this pull request Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ty_nil and replace with zero-length tuple
5 participants