-
Notifications
You must be signed in to change notification settings - Fork 26
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
Bool/Int type coercion #1170
base: dev
Are you sure you want to change the base?
Bool/Int type coercion #1170
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sakehl I'm not sure what isCPromoting does. Should I add one of the coercions as a case there? In principle, I want all coercions to work in C code, so I'm reluctant to add e.g. CoerceCIntBool as false
here, but it is not injective and thus not promoting...
Also, is it correct that some cases, like CoerceJoinUnion, call the regular isPromoting
instead of the special C version isCPromoting
? or is that just a copy-paste error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I wanted to capture here is that float/double can only be demoted
to int (and double to float) in:
This is called demotion, and is only valid in those cases. More specifically it is not valid with arithmetic conversions. (so 5 % 4.0
is not a valid statement, since 4.0 cannot be demoted to an int here, and % does not work on floating point numbers).
Anyway it seems that boolean is a bit harder. When looking specifically at if-statements and logical and/or statements of C:
https://en.cppreference.com/w/c/language/operator_logical
They work for an expression of any scalar type
. Which is of course not how we defined this internally in VerCors.
I think we can then best encode this to always allow demoting
pointers and integers to booleans.
So in that case, I'd say that we put:
case CoerceCIntBool() => false
case CoercePointerBool() => false
}
both to true.
(.... what is C a messy language)
Also, is it correct that some cases, like CoerceJoinUnion, call the regular isPromoting instead of the special C version isCPromoting? or is that just a copy-paste error?
This is definitely a copy paste error, thanks for spotting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can then best encode this to always allow demoting pointers and integers to booleans.
Ok, then I leave it as-is and let the default case handle it.
This is definitely a copy paste error
Ok, I'll fix it :)
A SYCL test (https://github.com/utwente-fmt/vercors/blob/9ad14fc3ba0c637eb79275df16d279044fcee850/examples/concepts/sycl/buffers/WrongGenericArgumentForConstructorHostdataType.cpp) fails in the CI. It is a negative test that tries to instantiate an int buffer with a bool array, which should fail. It still does, but now with a different error. @OmerSakar says it should still raise the original error, not the new one. The problem occurs, because at
I suspect that the SYCLTBufferImpl should not check the contained type of the array, but maybe @OmerSakar or @pieter-bos disagree? |
Before merging this PR, please check the following:
PR description
Add type coercion between
int
andbool
for C. Also coerce pointers tobool
.In C, integers can act as booleans (e.g. in
if(myInt)
), and booleans are just integers internally and can be passed as such, e.g. inTherefore, implicit casting between those types should work.
Pointers are basically integers, so the same applies for them. E.g. after
p=malloc(...)
, we might writeif(!p)return;
, treating the pointer as a boolean. This PR also covers that case.