Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upFixed regression in associated item resolution #28395
Conversation
rust-highfive
assigned
alexcrichton
Sep 13, 2015
This comment has been minimized.
This comment has been minimized.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
arielb1
reviewed
Sep 13, 2015
| ty_param_defs | ||
| .iter() | ||
| .map(|p| this.ty_infer(Some(p.clone()), Some(&mut substs), Some(TypeSpace), span)) | ||
| .map(|p| { |
This comment has been minimized.
This comment has been minimized.
arielb1
Sep 13, 2015
Contributor
This code rather needs some refactoring (at least, split it into another method).
ebfull
added some commits
Sep 14, 2015
This comment has been minimized.
This comment has been minimized.
|
r? @nrc |
rust-highfive
assigned
nrc
and unassigned
alexcrichton
Sep 14, 2015
arielb1
reviewed
Sep 14, 2015
| @@ -412,10 +412,26 @@ fn create_substs_for_ast_path<'tcx>( | |||
| // they were optional (e.g. paths inside expressions). | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Sep 14, 2015
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@bors: retry On Mon, Sep 14, 2015 at 6:39 AM, bors notifications@github.com wrote:
|
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Sep 15, 2015
This comment has been minimized.
This comment has been minimized.
bors
merged commit 4fec679
into
rust-lang:master
Sep 15, 2015
This comment has been minimized.
This comment has been minimized.
|
This is a 1.2-1.3 regression (#27591 is a runnable example) - may be worth backporting |
This comment has been minimized.
This comment has been minimized.
|
We're currently very close to 1.3 being released, so while not impossible it would be difficult to backport more PRs. With that in mind, it looks like this is just improving an ICE to be a first-class error message instead? Is there code that compiles on stable which does not compile on nightly right now? If it's just an improvement to the error message I'm inclined to not backport this as it's so close to the release, but it's not clear to me if that's the only part which was fixed. |
This comment has been minimized.
This comment has been minimized.
|
Yes, this only improves diagnostics. |
brson
added
I-needs-decision
beta-nominated
labels
Sep 15, 2015
This comment has been minimized.
This comment has been minimized.
|
cc @rust-lang/compiler @rust-lang/lang this was suggested for backporting to 1.3, which we are building today. Needs decision fast. |
This comment has been minimized.
This comment has been minimized.
|
Note that most of the code in this diff is tests. I am reasonably comfortable backporting. |
This comment has been minimized.
This comment has been minimized.
|
I recommend against backporting (this late). In the best case scenario this only affects diagnostics. Most of the code is tests, but this area of the code was not well tested before this change. |
This comment has been minimized.
This comment has been minimized.
|
I also don't feel comfortable backporting. |
brson
removed
the
I-needs-decision
label
Sep 15, 2015
This comment has been minimized.
This comment has been minimized.
|
The code in #27591 is an rpass example of this, but I guess too late. |
alexcrichton
added
T-lang
and removed
beta-nominated
T-lang
labels
Sep 23, 2015
This comment has been minimized.
This comment has been minimized.
|
Removing beta-nominated as the release was made |
ebfull commentedSep 13, 2015
Fixes #28344