Change default type parameters to match the behavior specified in the RFC #27761
Conversation
| .collect() | ||
| &TypeVariableValue::Bounded { .. } => Some(ty::TyVid { index: i as u32 }), | ||
| &TypeVariableValue::Known(v) => match v.sty { | ||
| ty::TyInfer(ty::FloatVar(_)) | ty::TyInfer(ty::IntVar(_)) => |
There was a problem hiding this comment.
Nit: the behavior of this fn merits a comment. Also, ty::TyInfer(_) seems more "forward compatible" to me (or else adding the other cases with a call to bug, since I think they are impossible)
|
After some discussion over IRC, @jroesch and I agreed current code isn't quite right -- in particular, the reason that user defaults wind up being preferred over integral defaults is because unsolved_variables just happens to return them in that order, which is quite non-obvious. My suggestion is to split the We must be sure to apply the user-defined defaults indiscriminantly (whether or not variable has since been bound) because otherwise conflicting defaults will go undetected. |
|
I also THINK that first loop could be simplified to something like this https://gist.github.com/nikomatsakis/a54823f42f8546436e7a, though naturally we'd want to use distinct sets as discussed in previous comment |
jroesch
force-pushed
the
default-typaram-user-defaults
branch
2 times, most recently
from
7cefd00 to
f9aff1e
Compare
|
Tidy is still unhappy. This looks good, modulo the question of what semantics we want. Can we rename things so that all the test cases have some common prefix, like "default-type-param-fallback"? |
jroesch
force-pushed
the
default-typaram-user-defaults
branch
from
0984c08 to
c9ecb8c
Compare
|
@bors r=nikomatsakis |
|
📌 Commit c9ecb8c has been approved by |
jroesch
force-pushed
the
default-typaram-user-defaults
branch
from
c9ecb8c to
2f5df9e
Compare
|
@bors r=nikomatsakis |
|
📌 Commit 2f5df9e has been approved by |
|
⌛ Testing commit 2f5df9e with merge b180375... |
|
Some cfail issues: https://travis-ci.org/rust-lang/rust/builds/78808560#L6472 |
|
@sfackler thanks! looks like I broke the error message formatting on my last commit, will fix and try again |
|
💔 Test failed - auto-mac-64-opt |
|
☔ The latest upstream changes (presumably #28274) made this pull request unmergeable. Please resolve the merge conflicts. |
The previous implementation was not very clear, this commit attempts to clarify and clean up the code that applies all defaults.
Thanks to @eddyb for bringing this bug to my attention.
jroesch
force-pushed
the
default-typaram-user-defaults
branch
from
2f5df9e to
bbcc665
Compare
|
☔ The latest upstream changes (presumably #28900) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Any updates on this PR? |
|
@steveklabnik this is my fault, things got a little busy with my move and starting my PhD, Niko was going to rebase this, but I assume he never found time. I'll get in contact with Niko and figure it out. |
|
Bah, I really ought to rebase this indeed. |
|
OK, so I pulled a copy of this branch locally and am going to close the PR. I'll follow up with @jroesch privately with a few questions. |
This addresses the issues raised by some members of the community about default priorities, and matches the behavior specified in the RFC.
You can also see the related internals discussion here
r? @nikomatsakis