Support _Bool as primitive type #47
Conversation
src/types.c
Outdated
| @@ -80,7 +80,7 @@ struct type | |||
| tp_short, tp_unsigned_short, | |||
| tp_int, tp_unsigned_int, | |||
| tp_long, tp_unsigned_long, | |||
| tp_long_long, tp_unsigned_long_long, | |||
| tp_long_long, tp_unsigned_long_long, tp_Bool, | |||
| /* Used as the rep type of enums whose constants are derived | |||
There was a problem hiding this comment.
- from C11: The rank of _Bool shall be less than the rank of all other standard integer types.
-> following the comment at the start of this enum, tp_Bool must be before tp_char
[And that also means there's a missing test case...]
There was a problem hiding this comment.
And some changes will likely follow elsewhere btw, I'm sure the code assumes tp_char is the lowest rank (e.g., see the assert in common_primitive_type).
There was a problem hiding this comment.
Also I would (without lots of thought admittedly) expect constant-folding of casts to have to implement:
"6.3.1.2 Boolean type
When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1"
[More generally: a change of this nature basically requires reading the portions of the C11 spec that specify the rules for _Bool]
|
Hi, |
src/cval.c
Outdated
| @@ -956,6 +956,9 @@ bool uint_inrange(largest_uint x, type t) | |||
| if (tsize == sizeof(largest_uint) && type_unsigned(t)) | |||
| return TRUE; | |||
|
|
|||
| if (type_Bool(t)) | |||
There was a problem hiding this comment.
Changing the *_inrange functions seems a) dangerous and b) likely the wrong place to achieve the desired effect.
I would remove the changes to these functions, and start by updating cval_cast to implement the cast-to-Bool rules - that will then automatically deal with, e.g., global initialisers (_Bool x = (void *) 0; or _Bool y = 2.3; e.g.). Some test cases (for compile-time-error vs no-compile-time-error) is then needed.
It does look like set_parameter_values in nesc-abstract.c should special-case bool, essentially replacing the special case in cval_inrange
nregress/ok/c99c11.bool.2
Outdated
| @@ -0,0 +1,2 @@ | |||
| In component `test': | |||
There was a problem hiding this comment.
This is a sign of something not right (a valid input should not be producing this warning).
src/unparse.c
Outdated
| @@ -343,6 +343,10 @@ void output_constant(known_cst c) | |||
| output_quoted_cs(ddecl->schars); | |||
| output("\""); | |||
| } | |||
| else if (type_pointer(t)) | |||
There was a problem hiding this comment.
This is also suspicious (and violates the "Requires:" clause on the function signature). I suspect it should not be needed with the cval_cast change.
There was a problem hiding this comment.
Currently only char[] and number is supported. To support null pointer as a constant, I added output_null_constant() function
|
Thank you for reviewing. Never mind, I also forgot this :) |
|
I pushed another try. |
Note that the regression test in this pull-request requires a proposed fix for
nesc.