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

Typedef chains via __ symbols cause errors #442

Closed
adetaylor opened this issue Apr 27, 2021 · 4 comments · Fixed by #449
Closed

Typedef chains via __ symbols cause errors #442

adetaylor opened this issue Apr 27, 2021 · 4 comments · Fixed by #449
Labels
bug Something isn't working

Comments

@adetaylor
Copy link
Collaborator

Build the s2 example with #438 and no generate! directives.

See:

unsupported type: pid_t", "unsupported type: FILE", "unsupported type: fpos_t", "unsupported type: clock_t", "unsupported type: time_t", "unsupported type: wint_t", "unsupported type: wctype_t", "unsupported type: mbstate_t", "unsupported type: ssize_t", "unsupported type: id_t", "unsupported type: siginfo_t", "unsupported type: dev_t", "unsupported type: mode_t", "unsupported type: pthread_attr_t", "unsupported type: pthread_t", "unsupported type: pthread_cond_t", "unsupported type: pthread_condattr_t", "unsupported type: pthread_mutex_t", "unsupported type: pthread_key_t", "unsupported type: pthread_mutexattr_t", "unsupported type: pthread_rwlock_t", "unsupported type: pthread_rwlockattr_t", "unsupported type: mach_port_t", "unsupported type: sigset_t", "unsupported type: wctrans_t", "unsupported type: nl_catd", "unsupported type: u128"

Rather than have a blocklist, I think we should ignore functions that refer to any type which isn't in known_types or specified in bindgen.

@adetaylor
Copy link
Collaborator Author

Example function:

 pub fn wait(arg1: *mut ::std::os::raw::c_int) -> root::pid_t;

@dtolnay
Copy link
Contributor

dtolnay commented Apr 27, 2021

Relevant cxx design sketch for supporting things like pid_t: dtolnay/cxx#529.

This was referenced Apr 28, 2021
@adetaylor
Copy link
Collaborator Author

This turns out to be nothing to do with anything above, and it's an interaction between #437 and #220.

Types undergo conversion from a bindgen format (e.g. root::std::unique_ptr<root::std::string>) to a cxx format (e.g. UniquePtr<CxxString>). This is done in a struct called TypeConverter.

The TypeConverter struct stores typedefs, as a map from type name -> syn::Type. The syn::Type is converted at insertion time. (Sometimes the syn::Type is another Type::Path, which could refer to another typedef.)

Then, when we encounter a function that uses a type which has a typedef name, it gets resolved recursively until we end up with something which isn't a typedef. That's what ends up in #[cxx::bridge].

Something similar happens with fields of POD structs, but I can't remember when this resolution occurs.

So far so... grudgingly OK.

The problem happens because in #437 TypeConverter now rejects types which contain __ because cxx doesn't like them. This needs to happen only for types which are actually going to be listed in the #[cxx::bridge] mod. That means the final thing to which the chain of typedefs actually resolves. In the case of the test in #445, pid_t is fine, and so is int to which it eventually resolves. Intermediate types are things like __int32_t or __darwin_pid_t which are not fine.

The specific issue is fixed here: https://github.com/google/autocxx/compare/double-underscore-typedefs-wip but this needs significant rework:

  • The API logging stuff needs to be cleaned up and abstracted to its own PR, hopefully with a way to autogenerate the enum variant descriptors.
  • There are now enough parameters to the various type conversion functions that I think we should introduce a TypeConversionOptions struct.
  • I'm not sure this fix is exactly right. There might be cases when we do need to check the __-compliance of idents in typedefs under some circumstances. Needs more thought.
  • TypeConverter needs nuking from orbit per Isolate TypeConverter into an analysis phase #220.

@adetaylor adetaylor changed the title Various fundamental unsupported C types Typedef chains via __ symbols cause errors Apr 28, 2021
@adetaylor adetaylor added the bug Something isn't working label Apr 28, 2021
@adetaylor
Copy link
Collaborator Author

Further problems encountered in #452.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants