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.

Sign up for GitHub

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

Jump to bottom

Add hint for people missing TryFrom, TryInto, FromIterator import pre-2021 #90288

Merged
merged 6 commits into from
Oct 27, 2021
Merged

Add hint for people missing TryFrom, TryInto, FromIterator import pre-2021 #90288

merged 6 commits into from
Oct 27, 2021

Conversation

JakobDegen
Copy link
Contributor

Adds a hint anytime a TryFrom, TryInto, FromIterator import is suggested noting that these traits are automatically imported in Edition 2021.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Oct 26, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@JakobDegen
Copy link
Contributor Author

JakobDegen commented Oct 26, 2021 •
edited
Loading

The issue here was that the suggestions are in the wrong order; the thing is this test passes locally. Probably the actual cause of this is that CI includes some ./x.py flag that I was not including locally, which eventually caused the ordering in rustc to give a different result. Making things #![no_std] hopefully fixed this (is that the best solution?). That being said, should this be considered a CI bug?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@davidtwco
Copy link
Member

davidtwco commented Oct 26, 2021 •
edited
Loading

The issue here was that the suggestions are in the wrong order; the thing is this test passes locally. Probably the actual cause of this is that CI includes some ./x.py flag that I was not including locally, which eventually caused the ordering in rustc to give a different result. Making things #![no_std] hopefully fixed this (is that the best solution?). That being said, should this be considered a CI bug?

Whenever I've seen an issue like this, it's typically some non-determinism somewhere in the compiler that happens to show up on CI. CI's output shows the command being run, so you should be able to confirm locally that it isn't a difference in flags or anything like that (but if it is, that might help you fix it). This sort of thing normally happens because there's a type with a non-deterministic iteration order that we're using somewhere, so you might be able to look for something like that (e.g. a HashMap or something like that). If you can't find the issue, then we can keep #[no_std] on the test, but ideally it wouldn't be necessary.

@davidtwco davidtwco 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 Oct 26, 2021
@JakobDegen
Copy link
Contributor Author

Change was caused by #89427 , I just hadn't rebased. Should be fixed now.

@JakobDegen
Copy link
Contributor Author

@rustbot label -S-waiting-on-author +S-waiting-on-review

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

@bors r+

@bors
Copy link
Contributor

bors commented Oct 27, 2021

📌 Commit cb336f1 has been approved by davidtwco

@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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 27, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 27, 2021
@bors
Auto merge of rust-lang#90347 - matthiaskrgr:rollup-rp2ms7j, r=matthi…
4e0d397 
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#90239 (Consistent big O notation in map.rs)
 - rust-lang#90267 (fix: inner attribute followed by outer attribute causing ICE)
 - rust-lang#90288 (Add hint for people missing `TryFrom`, `TryInto`, `FromIterator` import pre-2021)
 - rust-lang#90304 (Add regression test for rust-lang#75961)
 - rust-lang#90344 (Add tracking issue number to const_cstr_unchecked)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 83d5c24 into rust-lang:master Oct 27, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 27, 2021
@JakobDegen JakobDegen deleted the import_diagnostics branch November 11, 2021 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.58.0
Development

Successfully merging this pull request may close these issues.
6 participants
@JakobDegen @rust-highfive @rust-log-analyzer @davidtwco @bors @rustbot