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

inform user where to give a type annotation #48198

Merged
merged 4 commits into from
Feb 22, 2018

Conversation

csmoe
Copy link
Member

@csmoe csmoe commented Feb 14, 2018

should resolve #47777
previous pull request #47982 was closed because of a mistaken rebase.
r? @estebank

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@pietroalbini pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 14, 2018
@estebank
Copy link
Contributor

estebank commented Feb 14, 2018

@csmoe could you show the rustc output both before and after for any of the compile-fail errors? Also, I think you might have to rebase against latest master (git checkout master && git pull && git checkout inform_type_annotations && git rebase master) to get the correct behavior for two ui tests (otherwise the PR won't get merged by bors).

From the look of https://github.com/rust-lang/rust/pull/48198/files#diff-c3cb6b7128630f1f0a30c553e180e677, it tackles part of #47777, but I'd still like to keep that ticket open to extend the output with more information, like pointing where the type annotation should be added (for example).

@csmoe
Copy link
Member Author

csmoe commented Feb 15, 2018

@estebank
origin:

fn main() {
    let mut dirty_list = (0..5).collect();
    dirty_list.pop();
}

before

error[E0619]: the type of this value must be known in this context
 --> t.rs:3:5
  |
3 |     dirty_list.pop();
  |     ^^^^^^^^^^

error: aborting due to previous error

after this commit

error[E0282]: type annotations needed
 --> t.rs:3:5
  |
2 |     let mut dirty_list = (0..5).collect();
  |         -------------- consider giving `dirty_list` a type
3 |     dirty_list.pop();
  |     ^^^^^^^^^^ cannot infer type for `_`

error: aborting due to previous error

It’s really difficult for me to test the rustc on my laptop, I had tried about 9 hours to compile but never succeeded to build it when testing. So I just pulled this request to test my commits by bors(really sorry for the annoying failure message). Is there any way to compile with local llvm installed from linux packages repo?

I’ll update more changes about 2 days later to deal with the failure(Chinese New Year). sorry for the delay.

@estebank
Copy link
Contributor

after this commit:

Love it!

It’s really difficult for me to test the rustc on my laptop, I had tried about 9 hours to compile but never succeeded to build it when testing.

That is highly unusual. It takes me about 20 minutes to compile LLVM on first pull, and after that it doesn't need to be recompiled (until pulling changes to LLVM version from the repo).

So I just pulled this request to test my commits by bors(really sorry for the annoying failure message).

I do the same sometimes when working on multiple branches (wait for travis CI to fail or pass while I keep working).

Is there any way to compile with local llvm installed from linux packages repo?

I don't know, sorry :-/

I’ll update more changes about 2 days later to deal with the failure(Chinese New Year). sorry for the delay.

Don't worry! Take your time and enjoy the holiday. I like the change, I'm looking forward to having it merged. Just ping me once you've cleared the error.

@bors
Copy link
Contributor

bors commented Feb 16, 2018

☔ The latest upstream changes (presumably #46714) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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 Feb 16, 2018
@csmoe
Copy link
Member Author

csmoe commented Feb 18, 2018

error cleared.
cc @estebank

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 18, 2018

📌 Commit 4370a58 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 18, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 19, 2018
…tebank

inform user where to give a type annotation

should resolve rust-lang#47777
previous pull request rust-lang#47982 was closed because of a mistaken rebase.
r? @estebank
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 20, 2018
bors added a commit that referenced this pull request Feb 20, 2018
@nikomatsakis
Copy link
Contributor

So excited for this PR! 💯

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 21, 2018
…tebank

inform user where to give a type annotation

should resolve rust-lang#47777
previous pull request rust-lang#47982 was closed because of a mistaken rebase.
r? @estebank
bors added a commit that referenced this pull request Feb 22, 2018
Rollup of 12 pull requests

- Successful merges: #47379, #47833, #48106, #48198, #48314, #48325, #48335, #48352, #48354, #48360, #48382, #48397
- Failed merges:
@bors bors merged commit 4370a58 into rust-lang:master Feb 22, 2018
@bors
Copy link
Contributor

bors commented Feb 22, 2018

☔ The latest upstream changes (presumably #48399) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 22, 2018
@csmoe csmoe deleted the inform_type_annotations branch February 22, 2018 14:32
@nikomatsakis
Copy link
Contributor

@csmoe I don't know your twitter handle, so I'll just drop this here:

https://twitter.com/nikomatsakis/status/966737996434366464

@csmoe csmoe restored the inform_type_annotations branch February 23, 2018 01:35
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.

inform user where to give a type annotation after a call to collect
6 participants