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

Skip type and roundtrip tests for function types #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

valtog
Copy link

@valtog valtog commented Oct 30, 2019

Function types can mean pointers in Rust, but in C they are not interchangeable. This causes jemallocator to fail its test.

Function types can mean pointers in Rust, but in C they are not
interchangeable. This causes jemallocator to fail its test.
@gnzlbg
Copy link
Owner

gnzlbg commented Nov 4, 2019

@valtog thanks for the PR, what do you mean that they are not interchangeable ? Could you describe which problem does this solve ?

@valtog
Copy link
Author

valtog commented Nov 7, 2019

@gnzlbg Function pointers are like pointers (with some exception), so you can do

struct {
  void* (*f)(int a);
};

but function types are not: this doesn't compile (because C is not object-oriented)

struct {
  void* (f)(int a);
};

Likewise, this code generated by ctest

typedef struct {
  unsigned char c;
  extent_alloc_t v;
} type;

with extent_alloc_t defined in jemalloc as

typedef void *(extent_alloc_t)(extent_hooks_t *, void *, size_t, size_t, bool *,
  bool *, unsigned);

doesn't compile.

You might think of converting them to function pointers. But we don't even need to test function pointers, because it doesn't make sense to allocate or align them.

I want to fix this because I see jemallocator is stuck with an outdated version partly because of it (gnzlbg/jemallocator#130). I'm happy to tackle any other blocker for that PR.

@gnzlbg
Copy link
Owner

gnzlbg commented Nov 7, 2019

I'm not sure how this will fix that. When parsing a struct containing a function type in the jemalloc crate, we just see a struct Foo { x: extent_alloc_t } and have no idea whether extent_alloc_t is a function pointer or not at that point.

I think that the right fix for this is to add a cfg.is_fn_ty(|x| x == "extent_alloc_t"); method to the ctest builder, and then use that information when generating the tests.

@valtog
Copy link
Author

valtog commented Nov 8, 2019

@gnzlbg

(Never mind)

I'm not sure how this will fix that.

I've tested this with the jemallocator PR.

have no idea whether extent_alloc_t is a function pointer

I was proposing to remove tests for both.

I think that the right fix for this is to add a cfg.is_fn_ty(|x| x == "extent_alloc_t")

Well I'm fine with that too.

@valtog
Copy link
Author

valtog commented Nov 8, 2019

On second thought, we don't need cfg.is_fn_ty because we can detect that ourselves. See #86.

@gnzlbg
Copy link
Owner

gnzlbg commented Nov 8, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants