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

Treat refs to arrays the same as owned arrays in indexing_slicing and out_of_bounds_indexing #6034

Merged
merged 2 commits into from
Sep 16, 2020

Conversation

rail-rain
Copy link
Contributor

Fixes #6021

...and remove walk_ptrs_ty in favour of peel_refs, which came in 2019.


changelog: Fix a false positive in indexing_slicing and out_of_bounds_indexing where references to arrays weren't treated as arrays.

@rust-highfive
Copy link

r? @yaahc

(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 Sep 13, 2020
@bors
Copy link
Collaborator

bors commented Sep 15, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 16, 2020
@flip1995
Copy link
Member

@bors delegate+

r=me after rebasing (@bors r=flip1995)

TIL about peel_refs 👍

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

✌️ @rail-rain can now approve this pull request

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

📌 Commit c5a8b36 has been approved by flip1995)`

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout fix_fp_in_indexing_slicing (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self fix_fp_in_indexing_slicing --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
Auto-merging clippy_lints/src/utils/mod.rs
Auto-merging clippy_lints/src/methods/mod.rs
CONFLICT (content): Merge conflict in clippy_lints/src/methods/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

@flip1995
Copy link
Member

📌 Commit c5a8b36 has been approved by flip1995)`

Oh I thought putting it in a code block would prevent bors from triggering 🤔

treat refs to arrays the same as arrays
in `indexing_slicing` and `out_of_bounds_indexing`
@rail-rain
Copy link
Contributor Author

Thank you for the review! By the way, bors' instruction for resolving conflicts is slightly different from what I do usually. Especially I don't put -p on the rebase command. Is that OK?

@rail-rain
Copy link
Contributor Author

@bors r=flip1995

I've never used bors before. I hope this is what you mean ✌️.

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

📌 Commit ce06472 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

⌛ Testing commit ce06472 with merge 5af88e3...

@bors
Copy link
Collaborator

bors commented Sep 16, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 5af88e3 to master...

@bors bors merged commit 5af88e3 into rust-lang:master Sep 16, 2020
@flip1995
Copy link
Member

Especially I don't put -p on the rebase command.

Yeah same, I don't even know what -p does when rebasing. From the documentation:

-p
--preserve-merges
Recreate merge commits instead of flattening the history by replaying commits a merge commit introduces. Merge conflict resolutions or manual amendments to merge commits are not preserved.

This uses the --interactive machinery internally, but combining it with the --interactive option explicitly is generally not a good idea unless you know what you are doing (see BUGS below).

I don't think I'll use that in the future, since we usually want to prevent merge commits in PRs 🤔

I've never used bors before. I hope this is what you mean ✌️.

Perfect, thanks! 👍

@rail-rain rail-rain deleted the fix_fp_in_indexing_slicing branch September 17, 2020 07:40
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 from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

indexing_slicing displays error when indexing reference to array
5 participants