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

Codegen: promote C variadic args as needed #9747

Merged
merged 1 commit into from
Sep 29, 2020
Merged

Conversation

asterite
Copy link
Member

Fixes #9742

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:codegen labels Sep 14, 2020
call_arg = extend_float @program.float64, call_arg
elsif arg_type.is_a?(IntegerType) && arg_type.kind.in?(:i8, :u8, :i16, :u16)
# Integer with a size less that `int` must be converted to `int`
call_arg = extend_int arg_type, @program.int32, call_arg
Copy link
Member

Choose a reason for hiding this comment

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

So I guess technically this should check what the target platform C int type is, but in practice all platforms we support so far have int = int32_t. Just something to keep in mind in case somebody wants to target something weird or old.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I thought of this and reached the same conclusion. Also, I don't know how to know what's the target platform size of int.

Copy link
Contributor

@yxhuvud yxhuvud Sep 17, 2020

Choose a reason for hiding this comment

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

sizeof(LibC::Int) ? Or that is perhaps only the local compiler size of it?

Copy link
Member Author

@asterite asterite Sep 17, 2020

Choose a reason for hiding this comment

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

That's the size of the host machine. It won't work when cross-compiling.

@bcardiff bcardiff added this to the 1.0.0 milestone Sep 29, 2020
@asterite asterite merged commit bdbd386 into master Sep 29, 2020
@asterite asterite deleted the bug/promote-varargs branch September 29, 2020 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:codegen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crystal forgets to promote numbers when passing to ... va_args
5 participants