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

Reserve more numeric types. #907

Closed
wants to merge 2 commits into from
Closed

Reserve more numeric types. #907

wants to merge 2 commits into from

Conversation

mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Feb 25, 2015

@mahkoh
Copy link
Contributor Author

mahkoh commented Feb 25, 2015

Previous discussion: #138

@huonw
Copy link
Member

huonw commented Feb 25, 2015

What is this protecting against? That is, what's a program that will stop compiling if we don't reserve them?

It seems to me (and the filer of the original RFC) that shadowing should mean things work fine.

@mahkoh
Copy link
Contributor Author

mahkoh commented Feb 25, 2015

That is, what's a program that will stop compiling if we don't reserve them?

As mentioned in the RFC:

<anon>:1:1: 1:15 error: user-defined types or type parameters cannot shadow the primitive types [E0317]
<anon>:1 type u8 = u16;
         ^~~~~~~~~~~~~~


# Detailed design

Reserve the following type names: `fN`, `uN`, `iN` for `N` a multiple of 8.
Copy link
Member

Choose a reason for hiding this comment

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

What limit? Or just to, say, 232?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whatever the parser can handle.

Copy link
Member

Choose a reason for hiding this comment

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

The parser can handle arbitrarily large numbers: congruence mod 8 can be determined using only the last 3 digits of a number represented in base 10, meaning string handling works fine, and hence i1234567891234567891234567891234567800 could be checked to be "valid" under that rule.

Copy link

Choose a reason for hiding this comment

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

Is there any reason to limit ourselves to numbers that are a multiple of 8? Why not just reserve them for all numbers?

@Thiez
Copy link

Thiez commented Feb 25, 2015

Why take only multiples of 8 for N? If this change is going to be made we might as well take all of them, LLVM has some support for this.

@emberian
Copy link
Member

emberian commented Mar 2, 2015

That's a pretty annoying shadowing restriction right there, though it does avoid exposing the resolve bugs around such things...

@petrochenkov
Copy link
Contributor

@cmr
This restriction is temporary and will be removed if @eddyb 's plan works
See rust-lang/rust#20427 (comment)

@eddyb
Copy link
Member

eddyb commented Mar 3, 2015

👎 to this RFC as written, it's a really large hammer and only makes sense if we had arbitrarily sized primitives (like iN in LLVM, but we don't expose that - not that backends can deal with uncommon bit widths, anyways).

The only approach I know of and I consider (more or less) "correct" would be the one mentioned in the comment linked by @petrochenkov above.

That would protect against breakage by explicitly having all existing built-in types part of prelude::v1, which means f128 can be added backwards-compatibly, by requiring it to be manually imported from std::num (and only show up in prelude::v2, if ever).

That said, I almost forgot about it, and it requires associated constants to be done backwards-compatibly (or as close as we can get to that).

@bombless
Copy link

bombless commented Mar 3, 2015

We should fix the bug itself, not to shadow it with strange behavior.

@alexcrichton alexcrichton self-assigned this Mar 5, 2015
@alexcrichton
Copy link
Member

Thanks for the RFC! After some discussion, however, we don't think that this is the strategy that we would want to take. It is arguably a resolve bug which prevents shadowing primitives and we can consider it a requirement that adding a type like f16 would require fixing the resolve bug first to allow shadowing of primitives. As such the change should still be backwards compatible, just perhaps a little harder than before, so I'm going to close this.

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.

9 participants