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

Support casts in macros. #2268

Closed
wants to merge 5 commits into from
Closed

Conversation

reitermarkus
Copy link
Contributor

@reitermarkus reitermarkus commented Sep 9, 2022

Depends on jethrogb/rust-cexpr#33.

Built-in types are already working, but I don't know how to support custom types since types are parsed after macros.

I made evaluation of Var::ty for custom types lazy, but some types are still missing when generating the output for macro variables.

@reitermarkus reitermarkus force-pushed the cast-expr branch 8 times, most recently from ff2b297 to 5b3d8d1 Compare September 9, 2022 00:51
@reitermarkus reitermarkus force-pushed the cast-expr branch 3 times, most recently from 63cb97e to 56a4635 Compare September 9, 2022 01:27
@@ -354,6 +354,8 @@ pub struct BindgenContext {
/// This needs to be an std::HashMap because the cexpr API requires it.
parsed_macros: StdHashMap<Vec<u8>, cexpr::expr::EvalResult>,

wrapper_ids: StdHashMap<String, TypeId>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document this? It's not clear to me what you're trying to do here. Names can be namespaced in C++ too, which I think this wouldn't handle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, I need a way to get the type ID from a name, e.g.

typedef unsigned long MyType;
#define MY_CONST (MyType)123

So I need a way from "MyType" to c_ulong. Maybe this is possible using an existing method, but I couldn't find one.

let kind = match &ty[..] {
["bool"] => IntKind::Bool,
["char"] => IntKind::Char {
is_signed: value < 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems very weird to do this based on the value, can you elaborate? char is signed or unsigned depending on the platform.

Copy link
Contributor Author

@reitermarkus reitermarkus Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to go from "char" to the corresponding IntKind::Char with correct signedness? I guess this is a similar problem to the one with wrapper_ids above.

let var_ty = if let Some(var_ty) = self.ty(ctx) {
var_ty
} else {
// FIXME: Parse/output macro variables as the last step when all types are known.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, this is not possible, I believe, because the type the macro refers to by definition depends on context at the time of expansion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The time of expansion is after everything is parsed, so it should be the last type with the corresponding name.

Copy link
Contributor Author

@reitermarkus reitermarkus Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, bindgen currently is not doing the right thing for the following:

#define MY_INT 1
#define MY_INT 2

In C, MY_INT expands to 2. bindgen currently generates const MY_INT: u32 = 1.

So macros should be expanded/evaluated after parsing everything, not at the time we see them.

@bors-servo
Copy link

☔ The latest upstream changes (presumably #2284) made this pull request unmergeable. Please resolve the merge conflicts.

@reitermarkus reitermarkus requested a review from emilio October 16, 2022 16:42
@pvdrz pvdrz deleted the branch rust-lang:master November 2, 2023 17:50
@pvdrz pvdrz closed this Nov 2, 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 this pull request may close these issues.

4 participants