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

Lint unnamed address comparisons #5294

Merged
merged 1 commit into from
Mar 30, 2020
Merged

Lint unnamed address comparisons #5294

merged 1 commit into from
Mar 30, 2020

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Mar 9, 2020

Functions and vtables have an insignificant address. Attempts to compare such addresses will lead to very surprising behaviour. For example: addresses of different functions could compare equal; two trait object pointers representing the same object and the same type could be unequal.

Lint against unnamed address comparisons to avoid issues like those in rust-lang/rust#69757 and rust-lang/rust#54685.

changelog: New lints: [fn_address_comparisons] #5294, [vtable_address_comparisons] #5294

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 9, 2020
@bors
Copy link
Contributor

bors commented Mar 10, 2020

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

@tmiasko tmiasko changed the title Lint trait object pointer comparisons Lint unnamed address comparisons Mar 23, 2020
@bors
Copy link
Contributor

bors commented Mar 30, 2020

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

@flip1995 flip1995 requested a review from Manishearth March 30, 2020 17:49
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I'm unsure if "insignificant" is the best word to use here, though, thoughts on how to improve that?

Either way, please rebase and we can land this :)

@Manishearth Manishearth added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Mar 30, 2020
@tmiasko
Copy link
Contributor Author

tmiasko commented Mar 30, 2020

The issues generally arise from incorrect expectation that those addresses are unique, so "non-unique" might be another option.

@Manishearth
Copy link
Member

non-unique works for me! "unstable" is another one but that might give the impression that they're unstable across compiles, which isn't the main problem here

@Manishearth
Copy link
Member

@bors r+

thanks!

@bors
Copy link
Contributor

bors commented Mar 30, 2020

📌 Commit b77b219 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Mar 30, 2020

⌛ Testing commit b77b219 with merge 7c51e4e...

bors added a commit that referenced this pull request Mar 30, 2020
Lint unnamed address comparisons

Functions and vtables have an insignificant address. Attempts to compare such addresses will lead to very surprising behaviour. For example: addresses of different functions could compare equal; two trait object pointers representing the same object and the same type could be unequal.

Lint against unnamed address comparisons to avoid issues like those in rust-lang/rust#69757 and rust-lang/rust#54685.

Changelog: New lints: [`fn_address_comparisons`] [#5294](#5294), [`vtable_address_comparisons`] [#5294](#5294)
@bors
Copy link
Contributor

bors commented Mar 30, 2020

💔 Test failed - checks-action_test

@Manishearth
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Mar 30, 2020

⌛ Testing commit b77b219 with merge 0a25944...

@bors
Copy link
Contributor

bors commented Mar 30, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 0a25944 to master...

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) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants