Builder.derive_partialord(true) maybe should not derive PartialOrd for function pointers

Open
  • Is something wrong?
#3,360 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Feature
Clarity
Needs clarification
Activity status
Quiet
Tech stack
rust
Domain
devtools

Research direction

Start at the Builder::derive_partialord option and the code path that emits derives for structs containing function pointers. Reproduce the shown C declaration and generated Rust output, then inspect existing derive-related tests or add regression coverage; done means the intended behavior is documented and verified without the reported rustc warning.

Written by the indexing model from the issue text.

Description

Consider the following C declaration:

struct avc_memory_callback {
	void *(*func_malloc)(size_t size);
	/* other fields... */
};

Running bindgen on that, with a call to Builder.derive_partialord(true) (and other options) generates the following Rust declaration:

#[repr(C)]
#[derive(Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
pub struct avc_memory_callback {
    pub func_malloc:
        ::std::option::Option<unsafe extern "C" fn(size: usize) -> *mut ::std::os::raw::c_void>,
    /* other fields... */
}

The interesting point here is the #[derive(PartialOrd)], which causes the following rustc warning:

warning: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique
    --> <...>/selinux-sys/target/debug/build/selinux-sys-2e09f76ca98757af/out/selinux-sys.rs:1004:5
     |
1002 |   #[derive(Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
     |                                ---------- in this derive macro expansion
1003 |   pub struct avc_memory_callback {
1004 | /     pub func_malloc:
1005 | |         ::std::option::Option<unsafe extern "C" fn(size: usize) -> *mut ::std::os::raw::c_void>,
     | |_______________________________________________________________________________________________^
     |
     = note: the address of the same function can vary between different codegen units
     = note: furthermore, different functions could have the same address after being merged together
     = note: for more information visit <https://doc.rust-lang.org/nightly/core/ptr/fn.fn_addr_eq.html>
     = note: `#[warn(unpredictable_function_pointer_comparisons)]` on by default

The documentation of Builder::derive_partialord says:

Set whether the PartialOrd trait should be derived when possible.

And while this is technically possible, I think rustc here is requesting, politely, to avoid it. I agree with the assessment that rustc is showing. And my question is: should Builder.derive_partialord(true) still do it?

In the meantime, my fix for this issue is to call builder.no_partialeq("^avc_([_a-zA-Z0-9]+)_callback$") to manually exclude the types that contain function pointers. But, is this the reasonable fix for this situation, or should the behavior of Builder.derive_partialord(true) change to exclude function pointers?

Dominant language
Rust
Stars
5.3k
Forks
846
Avg merge
2d 6h
Merged PRs (30d)
11

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rust-lang/rust-bindgen

All issues in rust-lang/rust-bindgen

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.