librustc: Remove the fallback to int from typechecking.
#15026
Conversation
|
The data at https://gist.github.com/nikomatsakis/11179747 led me to believe that there were < 100 changes necessary to implement this system. With that in mind, why did this turn into a 3k line patch? Did the initial analysis not take into account all the test code? (which looks like where all the changes were). |
|
Presumably it didn't take the tests into account. |
|
Hmm, I don't recall, but it is likely I just "make rustc-stage1" or something like that. @alexcrichton does that affect your opinion on the change? (I don't think tests are especially representative.) |
|
@pcwalton Rebased code looks good but I didn't see any changes to the handling of enums; I remember some errors as a result. In meeting we discussed accepting "any integral type", did you change anything there? I'm sorry about the tests :/ |
|
My opinion is affected somewhat because while I don't think that tests are representative of the rest of our own codebase (libstd/librustc), tests are often representative of one-liners, first programs, and general first impressions with rust. This looks like in general small scripts and scraps of code are really suffering, but that may not be our target audience. It's kind of "one more thing" which is less ergonomic when you first start using rust perhaps? |
|
This was passing tests locally (at least before the rebase). |
|
I'm less sure about the change than I was, but I still lean toward doing it because (1) I don't believe we should be defaulting to 64-bit anything for performance reasons, and (2) the overflow implications of having the compiler automatically guess signed vs. unsigned are scary. |
|
Also (3) it will work better with user-defined numeric literals if we ever want to do that, because it makes any user-defined numeric type on equal footing with the built-in |
|
I pretty much agree with your reasoning 100% @pcwalton and was going |
|
(famous last words) |
| let buf = Buffer::new(self.log_size + delta); | ||
| // NB: not entirely obvious, but thanks to 2's complement, | ||
| // casting delta to uint and then adding gives the desired | ||
| // effect. |
There was a problem hiding this comment.
That's...actually nifty. I wonder if we should have a helper for this pattern:
fn offset(x: uint, offset: int) { x + offset as uint }
|
@pcwalton so this looks good, r+, but I'm still confused about what happened to enums. Did you wind up making any changes for discriminants? I couldn't find any when I looked but nor did I see any changes to mark discriminants as integers. |
|
@nikomatsakis I didn't see any issues with enums. I'm not quite sure what you meant by seeing errors there. |
|
@pcwalton interesting. So if you have something like this: I think that the type is never constrained. However, it may be that with the changes you've been making, we never try to resolve the resulting type to anything specific, and hence we never report any error. This is actually a perfectly good solution. |
This breaks a fair amount of code. The typical patterns are:
* `for _ in range(0, 10)`: change to `for _ in range(0u, 10)`;
* `println!("{}", 3)`: change to `println!("{}", 3i)`;
* `[1, 2, 3].len()`: change to `[1i, 2, 3].len()`.
RFC rust-lang#30. Closes rust-lang#6023.
[breaking-change]
| @@ -272,10 +292,20 @@ impl<'a> ResolveState<'a> { | |||
| Some(t) => ty::mk_mach_float(t), | |||
| None => { | |||
| if self.should(force_fvar) { | |||
| // As a last resort, default to f64. | |||
| // As a last resort, default to f64 and emit an error. | |||
There was a problem hiding this comment.
Why was this change performed? I see no mention of removing the f64 default in the associated RFC or the issue or anywhere else.
…Veykril fix: deduplicate fields and types in completion Fixes rust-lang#15024 - `hir_ty::autoderef()` (which is only meant to be used outside `hir-ty`) now deduplicates types and completely resolves inference variables within. - field completion now deduplicates fields of the same name and only picks such field of the first type in the deref chain.
…lnicola Deduplicate tuple indices for completion Follow-up to rust-lang#15026 A tuple struct may dereference to a primitive tuple (though unusual, which is why I previously overlooked this case). We should not show the same tuple index in completion in such cases. Deduplication of indices among multiple tuple structs is already handled in the previous PR.
Not yet ready for merge; lots of tests still failing.
This broke very little real code but broke pretty much every test everywhere.
cc @nikomatsakis