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

Allow specifying certain types we shouldn't derive/impl PartialEq for #965

Closed
8 tasks
fitzgen opened this issue Sep 7, 2017 · 2 comments
Closed
8 tasks

Allow specifying certain types we shouldn't derive/impl PartialEq for #965

fitzgen opened this issue Sep 7, 2017 · 2 comments
Labels
A-derive C-assigned E-less-easy enhancement help wanted

Comments

@fitzgen
Copy link
Member

fitzgen commented Sep 7, 2017

Add --no-partialeq <regex> and Builder::no_partialeq(String) to supply regexes for matching against types that we should not derive or implement PartialEq for.

Adding this feature involves:

  • 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-partialeq <regex> CLI flags into invocations of the builder method.

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

  • Tests!

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

    • When the no-partialeq type is explicitly whitelisted

    • When the no-partialeq type is marked opaque
@fitzgen fitzgen mentioned this issue Sep 8, 2017
Closed
5 tasks
@alexeyzab
Copy link
Contributor

I'd like to try adding this one.

@highfive: assign me

@highfive
Copy link

Hey @alexeyzab! Thanks for your interest in working on this issue. It's now assigned to you!

alexeyzab added a commit to alexeyzab/rust-bindgen that referenced this issue Sep 16, 2017
@alexeyzab
Add --no-partialeq <regex> flag
948108a 
Related to rust-lang#965.

- [x] Add 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-partialeq <regex> CLI flags into invocations of the builder method.

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

- [] Tests!

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

- [] When the no-partialeq type is explicitly whitelisted

- [] When the no-partialeq type is marked opaque
@alexeyzab alexeyzab mentioned this issue Sep 16, 2017
Merged
8 tasks
alexeyzab added a commit to alexeyzab/rust-bindgen that referenced this issue Sep 18, 2017
@alexeyzab
Abstract no-partialeq check
7c8f70b 
Fixes rust-lang#965
alexeyzab added a commit to alexeyzab/rust-bindgen that referenced this issue Sep 19, 2017
@alexeyzab
Add --no-partialeq <regex> flag
1917cb2 
Related to rust-lang#965.

- [x] Add 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-partialeq <regex> CLI flags into invocations of the builder method.

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

- [] Tests!

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

- [] When the no-partialeq type is explicitly whitelisted

- [] When the no-partialeq type is marked opaque
alexeyzab added a commit to alexeyzab/rust-bindgen that referenced this issue Sep 19, 2017
@alexeyzab
Abstract no-partialeq check
0105e4d 
Fixes rust-lang#965
alexeyzab added a commit to alexeyzab/rust-bindgen that referenced this issue Sep 19, 2017
@alexeyzab
Add --no-partialeq <regex> flag
e224ec0 
Related to rust-lang#965.

- [x] Add 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-partialeq <regex> CLI flags into invocations of the builder method.

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

- [] Tests!

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

- [] When the no-partialeq type is explicitly whitelisted

- [] When the no-partialeq type is marked opaque
alexeyzab added a commit to alexeyzab/rust-bindgen that referenced this issue Sep 19, 2017
@alexeyzab
Abstract no-partialeq check
7fb0d72 
Fixes rust-lang#965
bors-servo pushed a commit that referenced this issue Sep 19, 2017
Auto merge of #996 - alexeyzab:add-no-partialeq-command, r=fitzgen
4842973 
Add --no-partialeq <regex> flag

Related to #965.

- [x] Add 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-partialeq <regex> CLI flags into invocations of the builder method.

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

- [x] Tests!

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

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

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

This is my first pass at implementing this functionality, I haven't implemented the tests yet. I wanted to make sure I am on the right track, particularly when it comes to updating `MonotoneFramework::constrain`.

r? @fitzgen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-derive C-assigned E-less-easy enhancement help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fitzgen @alexeyzab @highfive