rust-lang / rust-lang/rust-bindgen

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

Open
#3,360 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
5.3k
Forks
829
Avg merge
1d 1h
Merged PRs (30d)
15

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?

Contributor guide

Open the contributing guide

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.

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.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.