Skip to content

/rust-bindgen

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

Conversation

Reviewers

@fitzgen fitzgen

Assignees

@fitzgen fitzgen

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@seemyvest @highfive @bors-servo @fitzgen
@seemyvest
Copy link
Contributor

seemyvest commented Oct 26, 2017 •
edited

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 highfive assigned fitzgen Oct 26, 2017

@highfive highfive added the S-awaiting-review label Oct 26, 2017

@highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

highfive commented Oct 26, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Oct 26, 2017

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

@highfive highfive added the S-needs-rebase label Oct 26, 2017

@fitzgen

fitzgen approved these changes Oct 26, 2017
View changes

Copy link
Member

fitzgen left a comment

Perfect! Thanks @seemyvest ! 👍
@fitzgen

This comment has been minimized.

Sign in to view
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

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Oct 26, 2017

✌️ @seemyvest can now approve this pull request
@seemyvest
Add --no-hash <regex> flag
Loading status checks…
5d27156

@seemyvest seemyvest force-pushed the seemyvest:issue-964 branch from 03eda28 to 5d27156 Oct 27, 2017

@seemyvest

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

seemyvest 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

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Oct 27, 2017

📌 Commit 5d27156 has been approved by seemyvest

@highfive highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Oct 27, 2017

@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Oct 27, 2017

⌛️ Testing commit 5d27156 with merge f315d2c...

bors-servo added a commit that referenced this pull request Oct 27, 2017

@bors-servo
Auto merge of #1105 - seemyvest:issue-964, r=seemyvest 
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
Loading status checks…
f315d2c
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Oct 27, 2017

☀️ 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
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@highfive highfive removed the S-awaiting-merge label Oct 27, 2017

@hallfox hallfox referenced this pull request Oct 27, 2017

Merged

give better variable name #1108

bors-servo added a commit that referenced this pull request Oct 27, 2017

@bors-servo
Auto merge of #1108 - hallfox:no-copy-name-patch, r=emilio 
give better variable name

Fixing up a comment from #1105
Loading status checks…
a08d8fd

@emilio emilio referenced this pull request Oct 28, 2017

Closed

Allow specifying certain types we shouldn't derive/impl Hash for #964

0 of 8 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.