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

Implement SliceIndex for R: RangeBounds<usize> #51464

Closed
wants to merge 1 commit into from

Conversation

joshlf
Copy link
Contributor

@joshlf joshlf commented Jun 9, 2018

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 9, 2018
@Mark-Simulacrum
Copy link
Member

r? @SimonSapin

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:08:42]    Compiling syntax_ext v0.0.0 (file:///checkout/src/libsyntax_ext)
[00:08:58] error[E0282]: type annotations needed
[00:08:58]     --> librustc/hir/lowering.rs:1903:58
[00:08:58]      |
[00:08:58] 1903 |                     add_bounds.get(&ty_param.id).map_or(&[][..], |x| &x),
[00:08:58]      |                                                          ^^^^^^ cannot infer type for `T`
41760 ./obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu
41756 ./obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release
41260 ./src/llvm/test/CodeGen/X86
40776 ./src/libcompiler_builtins

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@joshlf
Copy link
Contributor Author

joshlf commented Jun 10, 2018

@SimonSapin Do you have ideas on how to address the type inference issue (see the build log)?

@SimonSapin
Copy link
Contributor

No idea right now (and I don’t exactly understand what the error is exactly). However, what motivates this change? (But perhaps lets keep this aspect of the discussion on the tracking issue.)

@joshlf
Copy link
Contributor Author

joshlf commented Jun 10, 2018

I suspect it may be #46363; I've asked about it there. Re: motivation: I responded in the tracking issue.

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2018
@pietroalbini
Copy link
Member

Ping from triage! What's the status on this?

@SimonSapin
Copy link
Contributor

This likely cannot be landed as-is since it makes type inference ambiguous in some cases, including usage in rustc itself.

@TimNN
Copy link
Contributor

TimNN commented Jul 3, 2018

Ping from triage, @SimonSapin, @joshlf: If this cannot land as is, how should we move forward? Can this PR be fixed? Should we schedule a crater run? If neither of those, can this be closed?

@joshlf
Copy link
Contributor Author

joshlf commented Jul 3, 2018

@TimNN Is there a reason that it needs to be finalized within a given time frame? I am interested in fixing this, but that requires wrapping my head around some specialization issues that I don't understand, and so far, I haven't been able to find somebody who can explain them to me. I will keep trying, but I'm not sure how long it will take.

@TimNN
Copy link
Contributor

TimNN commented Jul 3, 2018

@joshlf: The release team regularly looks at all open pull requests, to make sure they make progress / are not forgotten (you can read more about the procedure at https://forge.rust-lang.org/triage-procedure.html).

In this specific case, as I understand the situation, this PR currently has some serious issues with no immediate plan to fix this. The last comment from you has also been over three weeks ago, which isn't necessarily a bad thing, however we can't know if the author has abandoned the PR or is still working on it. To avoid keeping stale PRs in the queue, we ping authors after some time of inactivity and close inactive PRs.

If you're still working on this, it's fine to keep this open.

I haven't been able to find somebody who can explain them to me. I will keep trying, but I'm not sure how long it will take.

I don't know if you have tried the following communication channels already, but I believe you could get some help there:

@joshlf
Copy link
Contributor Author

joshlf commented Jul 3, 2018

If you're still working on this, it's fine to keep this open.

Sounds good.

I don't know if you have tried the following communication channels already, but I believe you could get some help there...

Thanks! I'll try those channels.

@TimNN TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2018
@TimNN
Copy link
Contributor

TimNN commented Jul 17, 2018

Ping from triage, @joshlf ! Could you give us an update on your current plans for this PR?

@joshlf
Copy link
Contributor Author

joshlf commented Jul 17, 2018

I've been unable to figure out the issues. Is it feasible to re-open this PR in the future if I end up figuring it out? Because I don't want to hold up triage any more.

@pietroalbini
Copy link
Member

Sure, that's fine! You can close the PR, just remember not to force push before reopening it (GitHub is weird).

@joshlf joshlf closed this Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants