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

Add --no-hash <regex> flag #1105

Merged
merged 1 commit into from Oct 27, 2017
Merged

Add --no-hash <regex> flag #1105

merged 1 commit into from Oct 27, 2017

Conversation

ghost
Copy link

@ghost ghost commented Oct 26, 2017

Issue #964

  • Adding a new RegexSet member to bindgen::Builder (similar to the whitelisted_types set).

  • A Builder method to add strings to that RegexSet.

  • Plumbing in src/options.rs to convert --no-hash CLI flags into invocations of the builder method.

  • Making the MonotoneFramework::constrain function in src/ir/analysis/derive_hash.rs check if the given item is explicitly marked not to be Hash, and if so, inserting it into the self.cannot_derive_hash set via return self.insert(id).

  • Tests!

    • When the no-hash type is transitively referenced by a whitelisted item

    • When the no-hash type is explicitly whitelisted

    • When the no-hash type is marked opaque

r? @fitzgen

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@bors-servo
Copy link

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

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks @seemyvest ! 👍

@fitzgen
Copy link
Member

fitzgen commented Oct 26, 2017

@bors-servo delegate+

Ah, looks like some merge conflicts snuck in. Once you've rebased and resolved them you can tell r+ to @bors-servo yourself.

Thanks again!

@bors-servo
Copy link

✌️ @seemyvest can now approve this pull request

@ghost
Copy link
Author

ghost commented Oct 27, 2017

No problem :) @bors-servo r+
I noticed while doing this that a misnamed variable was missed in #1099 here. no_partialeq should be named no_copy.

I'll have a look and see what others I can do, feel free to point some out to me :)

@bors-servo
Copy link

📌 Commit 5d27156 has been approved by seemyvest

@bors-servo
Copy link

⌛ Testing commit 5d27156 with merge f315d2c...

bors-servo pushed a commit that referenced this pull request Oct 27, 2017
Add --no-hash <regex> flag

Issue #964

- [x] Adding a new RegexSet member to bindgen::Builder (similar to the whitelisted_types set).

- [x] A Builder method to add strings to that RegexSet.

- [x] Plumbing in src/options.rs to convert --no-hash <regex> CLI flags into invocations of the builder method.

- [x] Making the MonotoneFramework::constrain function in src/ir/analysis/derive_hash.rs check if the given item is explicitly marked not to be Hash, and if so, inserting it into the self.cannot_derive_hash set via return self.insert(id).

- [x] Tests!

  - [x] When the no-hash type is transitively referenced by a whitelisted item

  - [x] When the no-hash type is explicitly whitelisted

  - [x] When the no-hash type is marked opaque

r? @fitzgen
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: seemyvest
Pushing f315d2c to master...

@bors-servo bors-servo merged commit 5d27156 into rust-lang:master Oct 27, 2017
bors-servo pushed a commit that referenced this pull request Oct 27, 2017
give better variable name

Fixing up a comment from #1105
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants