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

Macro redefinitions are not being respected #2472

Open
pvdrz opened this issue Mar 29, 2023 · 1 comment · Fixed by #2543
Open

Macro redefinitions are not being respected #2472

pvdrz opened this issue Mar 29, 2023 · 1 comment · Fixed by #2543

Comments

@pvdrz
Copy link
Contributor

pvdrz commented Mar 29, 2023

Input C/C++ Header

#define FOO 4
#define BAR (1 + FOO)
#undef FOO
#define FOO 5
#define BAZ (1 + FOO)

Bindgen Invocation

$ bindgen input.h

Actual Results

pub const FOO: u32 = 4;
pub const BAR: u32 = 5;
pub const BAZ: u32 = 6;

Expected Results

I think the actual results are inconsistent because BAZ was evaluated with the redefinition of FOO but FOO itself was not. For the sake of consistency I'd say that the expected behavior should be one of the following:

  • The values of FOO, BAR and BAZ are computed using the first definition of FOO. Meaning that the second definition of FOO is ignored.
pub const FOO: u32 = 4;
pub const BAR: u32 = 5;
pub const BAZ: u32 = 5;
  • Or even better, the value of BAR is computed the first definition of FOO and the values of FOO and BAZ are computed using the second definition of FOO.
pub const BAR: u32 = 5;
pub const FOO: u32 = 5;
pub const BAZ: u32 = 6;
  • Alternatively, all the macros could be "expanded/evaluated" after reading all the file. Meaning that all the values are computed with the last definition of FOO:
pub const BAR: u32 = 6;
pub const FOO: u32 = 5;
pub const BAZ: u32 = 6;

I'd say that there is no "right" answer but I'd prefer the second or third option.

@pvdrz pvdrz linked a pull request Jun 8, 2023 that will close this issue
@pvdrz
Copy link
Contributor Author

pvdrz commented Aug 9, 2023

We had to revert this change so I'm reopening this.

@pvdrz pvdrz reopened this Aug 9, 2023
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 a pull request may close this issue.

1 participant