Factor out the collection and traversal of the transitive closure of whitelisted items #184
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ah, and you beat me to the punch with the |
fitzgen
force-pushed
the
whitelisted-items
branch
from
2d25a43 to
ca99ef3
Compare
|
@emilio ok, rebased! |
emilio
reviewed
Nov 1, 2016
| .map(|(&id, _)| id); | ||
|
|
||
| let seen: ItemSet = roots.collect(); | ||
| let to_iterate = seen.iter().cloned().collect(); |
There was a problem hiding this comment.
So it seems we can preserve the original order of iteration (which would preserve a sort of nicer order) reversing this, is that right?
There was a problem hiding this comment.
Let me check really quick.
| { | ||
| ctx: &'ctx BindgenContext<'gen>, | ||
|
|
||
| // The set of whitelisted items we have seen. If you think of traversing |
There was a problem hiding this comment.
Maybe want this to be a doc comment?
There was a problem hiding this comment.
It isn't pub so I'm not sure that it matters -- do you feel strongly about it?
| if !item.is_toplevel(context) { | ||
| continue; | ||
| } | ||
| let whitelisted_items: ItemSet = context.whitelisted_items().collect(); |
There was a problem hiding this comment.
Do we really need to collect this? Seems like lazily iterating over it should work? Or do we modify the context at this point?
There was a problem hiding this comment.
Oh, I see, non-whitelisted ancestors.
There was a problem hiding this comment.
Yes, because checking if the ancestor is in the set requires that we have fully computed the set.
| } | ||
| } | ||
|
|
||
| return Some(id); |
There was a problem hiding this comment.
nit: you can drop the return and just use Some(id).
This commit adds the `BindgenContext::whitelisted_items` method and `WhitelistedItemsIter` iterator. Together, they can be used to iterate over whielisted items' transitive closure.
This replaces the manual gathering and traversal of the transitive closure of whitelisted items with the new canonical method.
fitzgen
force-pushed
the
whitelisted-items
branch
from
ca99ef3 to
5691cab
Compare
|
@emilio fixed up review comments |
|
Yes, reversing the initial roots preserved the old order :) |
|
@bors-servo r+ Nice! |
|
📌 Commit 5691cab has been approved by |
bors-servo
pushed a commit
that referenced
this pull request
Nov 1, 2016
Factor out the collection and traversal of the transitive closure of whitelisted items I'm going to re-use this stuff, plus I think it is way more understandable when pulled out of `codegen`. This did have the unfortunate consequence of changing iteration order of types, resulting in different anonymous bindgen names for things like unions and such. We *could* sort the whitelisted item set before codegen to alleviate this kind of thing in the future, but I'm not sure it is worth the extra overhead... r? @emilio
|
☀️ Test successful - status-travis |
luser
pushed a commit
to luser/rust-bindgen
that referenced
this pull request
Mar 27, 2017
Generate better enums Squash of... Disable prefixing Default to failing on unknown types Add support for Char16 Emit errors for unknown cursor kinds Hack in support for classes Recurse into unexposed decls This fixes functions that use extern "C". Add support for generating unmangled functions Prefix unnamed data structures with the file name Recurse into namespaced things Avoid rust reserved keywords in unmangle func args Don't create variadic unmangled funcs Don't translate typedefs to the same name Ignore operator overloads Avoid templates Handle class declarations Number duplicate demangle functions Implement copy on enums Translate stdint types into standard rust int types Switch enums to i32 for better compatibility Correctly deal with mangled functions with unnamed args Mark unmangling functions as unsafe Attempt to produce structs for C++ classes Convert references Generate better func decls for void returns Make every function callback unsafe Add support for generics in typedefs Add support for class templates Aggressively trim duplicates Don't generate default impl for templates Improve handling of templates Fix function numbering Fix deduplication Make unmangling functions extern "C" Convert all int/float typedefs to standard rust ints/floats This also gives better information to the bitfield parsing and allows uint32_t and other stdint bitfields to be processed properly Add support for wchar Add support for bitfield setter generation Fix floats Squash of... Shorten generated bitfield names Add support for generating whole bitfields Add support for enums nested inside structs/classes Rustup Fixes rust-lang#184. Rustup to b301e02f3 2015-05-19 Inline demangling functions Add support for base classes/types Generate bindings for methods Make duplicate detection less aggressive Avoid converting long/unsigned longs to rust types. This fixes 64/32bit issues in structs. Generate bitfields correctly for typedefs Convert stdint types to rust types Derive Debug on BindgenOptions, Bindings, and LinkType. Remove +'static when writing bindings Generate virtual function tables Resolve some warnings Add NotConst where Constness params are required Generate real bools when applicable Squash of... Add support for comments Improve bitfield support using const fn Add limited support for references Add comments to fields Don't generate empty comments Convert char16_t to u16 rather than i16 Convert signed chars to libc::c_char Fix Cargo.toml rebasing breakage Fix compiler errors This gets bindgen to compile and run again, but all but one `cargo test` tests fail. Not sure if that’s because of mistakes on my part or if the sm-hacks branch never passed those tests. Fix build warnings Use link_name attr for mangled function names Handle mangled global vars Only generate bindings for visible symbols Don't generate arrays for blobs when the length is 1 Name enums inside classes better Handle template args inside typedefs better Filter out duplicate decls better Generate correctly sized enums Squash of... Fix bitfield accessor method generation for bools Insert phantom fields in empty structs Don't mark unmangling methods as extern "C" Add back comment support for functions Add basic annotation support Don't generate univariant enums Add support for hide annotation and adjust syntax Don't generate unsupported structs Don't parse hidden fields Don't derive Copy for structs with destructors Don't implement Clone or Default Derive Clone when deriving Copy Bypass single member unions Disable references in function args for now Remove extra underscore in mangled names on OSX Don't translate private methods Support generating consts from macros that are defined as integer literals. Handle mangling better Squash of... Update README.md for fork Generate docs for enum items Generate docs for typedefs Generate docs for enums Update syntex_syntax to 0.24.* Update clang info in README.md Spit errors and warnings to stdout. The correct thing to do here is to use env_logger, but that was causing cargo troubles for me, and this is preferable to swallowing them. Add the -ignore-functions argument. Handle 1 << 63 as enum value. Don't try to convert standard int types in rust_type_id. It looks like mwu added this, but I'm pretty sure it's a category error. This function appears to be designed to reproducibly permute C identifiers so that they don't conflict with builtin rust types. It's specifically _not_ a type translator (which would happen at the type level, rather than the string level), and using it as such with callers like ctypedef_to_rs causes us to generate things like: type u64 = u64; While processing stdint.h, which is clearly wrong. Stop patching in placeholder names for CompInfo and EnumInfo instances during code generator. As best as I can tell, it's done this way (rather than my way) because bindgen tries to recognize struct and enums typedefs of the form: /* This is a common idiom in C, not so much in C++ */ typdef struct { ... } Foo; The intention, I think, is to avoid generating rust code for a struct with a placeholder name followed by a typedef, and just give the struct the right name initially. This seems like a reasonable goal, though not a particularly important one. However, in my testing this never actually happens, because we end up calling unnamed_name anyway during the GComp(ci) case of gen_mod before we get to evaluting the typedef. So let's just remove that stuff and simplify the code. This lets us remove all the borrow_mut calls during code generation, which seems necessary for soundness. gen: Allow empty union members Use full paths in generation. Fix test compilation parser: Add support for parsing namespaces Partial C++ namespaces support We currently generate the modules as expected, but we don't resolve the names from external namespaces well. Remove unnecesary return statements Put namespaces behind a flag Overall now that they aren't complete still. Moar refactoring Finally take rid of all the warnings Even moar gen: Avoid so much cloning parser: Refactor the way submodules are stored This way we can share the global map, while having each module custom globals. gen: Checkpoint before the refactoring This actually keeps working as before. gen: Make modules (almost) work for typedef'd types We generate almost valid code, we just have to add some use statements. Or maybe is a better idea to add an unintelligible name to the root mod, and actually output a root mod plus a use root::* before. gen: Document the current submodule approach and some desirable alternative gen: Make it actually compilable \o/ gen: Make the code generation usable for every type. There's just an edge case I've detected, and it's when we remove the instantiation of C<int>, and another module uses it, because that means we only know of its existance in that other module. Probably we might want to use cursor_getSemanticParent to get the real class instead of the instantiated, but I'm not too confident about that. Fix a corner case when a template was instantiated in another module. Added an example of the namespace resolution. Don't panic when not finding the specialised template This can be annoying if filtering files out. Straight rebase completed, let's fix that build errors wip Pair up with master nits Update AST Add -no-rename-fields option This is for compatibility between C bindings and C++ bindings (in C `struct Foo` and `enum Foo`, are different, while in C++ they aren't). wip Add enum tests pass, and add C++ tests Make a few more struct-related tests pass
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm going to re-use this stuff, plus I think it is way more understandable when pulled out of
codegen.This did have the unfortunate consequence of changing iteration order of types, resulting in different anonymous bindgen names for things like unions and such. We could sort the whitelisted item set before codegen to alleviate this kind of thing in the future, but I'm not sure it is worth the extra overhead...
r? @emilio