rust-lang / rust-lang/libc

ctest support for conflicting headers

Open
#5,344 13 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

ctest
Dominant language
Rust
Stars
2.6k
Forks
1.3k
Avg merge
1d 22h
Merged PRs (30d)
69

Description

EDIT: this may not be the best proposal, see discussion below.


We have a few cases where we bind multiple symbols from headers that can't be included together:pidfd is one example https://github.com/rust-lang/libc/pull/5191, https://github.com/rust-lang/libc/pull/5326 will add another. I think it might be possible for ctest to add this reasonably easy, rough sketch:

struct TestGenerator {
    // Move fields like `headers`, `skip`, etc into a reusable struct
    pub(crate) default_group: TestGroupInner,
    pub(crate) groups: Vec<TestGroup>, // new
    // ... existing
}

#[derive(Clone)]
struct TestGroup(Rc<RefCell<TestGroupInner>>);

struct TestGroupInner {
    pub(crate) name: BoxStr,
    pub(crate) headers: ..., // Same as in TestGenerator
    pub(crate) skips: Vec<Skip>,
}

impl TestGenerator {
    // Name must match regex `\p{xid_start}\p{xid_continue}*` (i.e. valid identifier)
    // and must be unique, else panic
    fn group(&mut self, name: &str) -> TestGroup;
    fn groups(&self) -> impl Iterator<Item = TestGroup>;
}

impl TestGroup {
    fn name(&self) -> &str;

    // Same signatures as `TestGenerator`. 
    fn define(...) -> &mut Self;
    fn header(...) -> &mut Self;
    fn header_with_defines(...) -> &mut Self;
    fn skip_struct(&mut self, f: impl Fn(&Struct) -> bool + 'static) -> &mut Self;
    // ... skip_*

    fn matches_struct(&mut self, f: impl Fn(&Struct) -> bool + 'static) -> &mut Self;
    fn matches_static(&mut self, f: impl Fn(&Static) -> bool + 'static) -> &mut Self;
    // ... other types
}

For users:

  1. If you have headers that may conflict or want to split the test up for some reason, create a new group
  2. Headers and defines can be set for a specific group. Defines are also inherited from the default group
  3. If some API should be tested as part of a group, use a matches_* function. This excludes it from the default group

For ctest:

  1. Each group gets a separate .c file, including one for the "default" group
  2. If not the default group, put the group name in the function names so they don't conflict. E.g. ctest_group1_const_red

This doesn't help us with nested modules but does give us a start. (I think those can be handled with just a fn path(&self) -> &str for Struct, Union, etc.)

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 by reading the discussion and the proposed TestGenerator, TestGroup, and TestGroupInner interfaces in the issue. Resolve whether this design is still wanted, then trace ctest's current generator behavior before defining the group API. Done means conflicting headers can be assigned to separate groups, each group produces its own C file, and generated names do not conflict.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
testing-qa, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.