All DEF constants are now unsigned?
Hello Cython-devel, I noticed that now unsigned constants like DEF foo = 1 become 1U in the C source code. This change (i.e. commit ea6414cd293ba2134c3df5b37b158d8e3db79083) broke Sage. I haven't managed yet to actually isolate why this change broke something. Was this change of 1 to 1U intentional? Why was it done? Jeroen.
This is a bug: DEF myconst = 1 def foo(): cdef long x = -myconst return x This returns 4294967295.
Jeroen Demeyer schrieb am 05.09.2015 um 12:17:
I noticed that now unsigned constants like
DEF foo = 1
become 1U in the C source code.
This change (i.e. commit ea6414cd293ba2134c3df5b37b158d8e3db79083) broke Sage. I haven't managed yet to actually isolate why this change broke something.
Was this change of 1 to 1U intentional? Why was it done?
Pretty much intentional, yes. I changed it in response to this bug report: http://thread.gmane.org/gmane.comp.python.cython.devel/15774 The problem with DEF constants is that they are Python values that use Python semantics in expressions before they eventually get converted back into literals and inserted into the AST. Specifically, they loose any literals semantics that they initially had in the source code, including any information about literal C integer types that contributed to their value, as in this case. Thus, there isn't really a 'correct' way to write them out as literals and C might be happy with what we output, or not. I guess it might be better to undo this part of the change. Unsigned literals are usually not the right way to do it in C, definitely not for everything. We could then maybe apply the usual 32/64 bits heuristic to add at least L/LL suffixes at need. Stefan
Stefan Behnel schrieb am 05.09.2015 um 16:01:
Jeroen Demeyer schrieb am 05.09.2015 um 12:17:
I noticed that now unsigned constants like
DEF foo = 1
become 1U in the C source code.
This change (i.e. commit ea6414cd293ba2134c3df5b37b158d8e3db79083) broke Sage. I haven't managed yet to actually isolate why this change broke something.
Was this change of 1 to 1U intentional? Why was it done?
Pretty much intentional, yes. I changed it in response to this bug report:
http://thread.gmane.org/gmane.comp.python.cython.devel/15774
The problem with DEF constants is that they are Python values that use Python semantics in expressions before they eventually get converted back into literals and inserted into the AST. Specifically, they loose any literals semantics that they initially had in the source code, including any information about literal C integer types that contributed to their value, as in this case. Thus, there isn't really a 'correct' way to write them out as literals and C might be happy with what we output, or not.
I guess it might be better to undo this part of the change. Unsigned literals are usually not the right way to do it in C, definitely not for everything. We could then maybe apply the usual 32/64 bits heuristic to add at least L/LL suffixes at need.
Does this look better? https://github.com/cython/cython/commit/2ac9ffbab1e868137bf33bf025eb7a5c7e47... It now appends the 'U' suffix only to literals that are used in an unsigned context (e.g. assignment to unsigned variable), and additionally appends L/LL suffixes for integers that might exceed the int/long limits. That's just guessing, but it should generally improve the current handling of literals and DEF expressions. Stefan
On 2015-09-05 17:09, Stefan Behnel wrote:
It now appends the 'U' suffix only to literals that are used in an unsigned context (e.g. assignment to unsigned variable), and additionally appends L/LL suffixes for integers that might exceed the int/long limits.
Why not just keep it simple, append no suffixes and let the C/C++ compiler deal with it? No heuristics needed. More concretely, I would handle this as follows: * do not use any L or U suffix * write all DEF constants as hexadecimal in the source code This way, the C/C++ compiler will determine the integral type. It will use the first type in the following list which can represent the number: - int - unsigned int - long - unsigned long - long long - unsigned long long The reason for the *hexadecimal* constants is that the unsigned types are not tried for decimal constants.
Jeroen Demeyer schrieb am 06.09.2015 um 10:54:
On 2015-09-05 17:09, Stefan Behnel wrote:
It now appends the 'U' suffix only to literals that are used in an unsigned context (e.g. assignment to unsigned variable), and additionally appends L/LL suffixes for integers that might exceed the int/long limits.
Why not just keep it simple, append no suffixes and let the C/C++ compiler deal with it? No heuristics needed.
More concretely, I would handle this as follows: * do not use any L or U suffix * write all DEF constants as hexadecimal in the source code
This way, the C/C++ compiler will determine the integral type.
I previously tried that with decimal literals and got warnings in gcc. Hadn't tried hex literals.
It will use the first type in the following list which can represent the number: - int - unsigned int - long - unsigned long - long long - unsigned long long
The reason for the *hexadecimal* constants is that the unsigned types are not tried for decimal constants.
That's interesting to know (you were quoting 6.4.4 of the C standard apparently). C - what a language full of wonders. https://github.com/cython/cython/commit/37f6b07978d3ca1b41aae4d1dd747ee63e3b... Thanks! Stefan
On Sun, Sep 6, 2015 at 5:30 AM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Jeroen Demeyer schrieb am 06.09.2015 um 10:54:
On 2015-09-05 17:09, Stefan Behnel wrote:
It now appends the 'U' suffix only to literals that are used in an unsigned context (e.g. assignment to unsigned variable), and additionally appends L/LL suffixes for integers that might exceed the int/long limits.
Why not just keep it simple, append no suffixes and let the C/C++ compiler deal with it? No heuristics needed.
More concretely, I would handle this as follows: * do not use any L or U suffix * write all DEF constants as hexadecimal in the source code
This way, the C/C++ compiler will determine the integral type.
I previously tried that with decimal literals and got warnings in gcc. Hadn't tried hex literals.
It will use the first type in the following list which can represent the number: - int - unsigned int - long - unsigned long - long long - unsigned long long
The reason for the *hexadecimal* constants is that the unsigned types are not tried for decimal constants.
That's interesting to know (you were quoting 6.4.4 of the C standard apparently). C - what a language full of wonders.
Wow, I didn't know that either... thanks for the pointer.
participants (3)
-
Jeroen Demeyer -
Robert Bradshaw -
Stefan Behnel