* [PATCH] Make large enum constants unsigned
@ 2014-05-22 5:30 Stefan Kristiansson
2014-05-22 6:22 ` Maciej W. Rozycki
2014-05-22 15:11 ` Frank Ch. Eigler
0 siblings, 2 replies; 14+ messages in thread
From: Stefan Kristiansson @ 2014-05-22 5:30 UTC (permalink / raw)
To: cgen
Cc: Alan Modra, fche, Christian Svensson, Pierre Muller, Maciej W. Rozycki
Hi,
this fixes a bug noted in two threads on the binutils mailing list:
https://sourceware.org/ml/binutils/2014-05/msg00152.html
https://sourceware.org/ml/binutils/2014-05/msg00195.html
Both threads describe the problem pretty well, but the gist of it
is that constants are generated that will be interpreted as signed.
2014-05-22 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
* enum.scm (gen-enum-decl): Emit 'ULL' after constants larger than
#x80000000
Index: cgen/enum.scm
===================================================================
RCS file: /cvs/src/src/cgen/enum.scm,v
retrieving revision 1.13
diff -u -r1.13 enum.scm
--- cgen/enum.scm 13 Feb 2010 03:39:15 -0000 1.13
+++ cgen/enum.scm 22 May 2014 05:23:23 -0000
@@ -298,7 +298,10 @@
""
(string-append " = "
(if (number? (cadr e))
- (number->string (cadr e))
+ (string-append (number->string (cadr e))
+ (if (> (cadr e)
+ #x80000000)
+ "ULL" ""))
(cadr e))))
))
(if (and san? include-sanitize-marker?)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 5:30 [PATCH] Make large enum constants unsigned Stefan Kristiansson
@ 2014-05-22 6:22 ` Maciej W. Rozycki
2014-05-22 6:52 ` Stefan Kristiansson
2014-05-22 15:11 ` Frank Ch. Eigler
1 sibling, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2014-05-22 6:22 UTC (permalink / raw)
To: Stefan Kristiansson
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller
On Thu, 22 May 2014, Stefan Kristiansson wrote:
> this fixes a bug noted in two threads on the binutils mailing list:
> https://sourceware.org/ml/binutils/2014-05/msg00152.html
> https://sourceware.org/ml/binutils/2014-05/msg00195.html
>
> Both threads describe the problem pretty well, but the gist of it
> is that constants are generated that will be interpreted as signed.
>
> 2014-05-22 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>
> * enum.scm (gen-enum-decl): Emit 'ULL' after constants larger than
> #x80000000
Well, `ULL' is non-standard for pre-C99 compilers and also not needed
because an enum will never have a type that is wider than `int'. So for
portability's sake I suggest that you use `U' or `u' as I proposed (I
prefer lowercase `u' for decimal constants because I find it easier to
spot among digits, however please feel free to take your pick).
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 6:22 ` Maciej W. Rozycki
@ 2014-05-22 6:52 ` Stefan Kristiansson
2014-05-22 7:10 ` Stefan Kristiansson
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Kristiansson @ 2014-05-22 6:52 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Stefan Kristiansson, cgen, Alan Modra, fche, Christian Svensson,
Pierre Muller
On Thu, May 22, 2014 at 9:21 AM, Maciej W. Rozycki
<macro@codesourcery.com> wrote:
> On Thu, 22 May 2014, Stefan Kristiansson wrote:
>
>> this fixes a bug noted in two threads on the binutils mailing list:
>> https://sourceware.org/ml/binutils/2014-05/msg00152.html
>> https://sourceware.org/ml/binutils/2014-05/msg00195.html
>>
>> Both threads describe the problem pretty well, but the gist of it
>> is that constants are generated that will be interpreted as signed.
>>
>> 2014-05-22 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>>
>> * enum.scm (gen-enum-decl): Emit 'ULL' after constants larger than
>> #x80000000
>
> Well, `ULL' is non-standard for pre-C99 compilers and also not needed
> because an enum will never have a type that is wider than `int'. So for
> portability's sake I suggest that you use `U' or `u' as I proposed (I
> prefer lowercase `u' for decimal constants because I find it easier to
> spot among digits, however please feel free to take your pick).
>
Hmm, right.
I actually had a plain 'U' in my first version of the patch, but then
changed it to 'ULL' last minute with the thought of being future proof
against possible larger mask values.
But as you point out, that will have issues anyway, so I'll repost
with the original patch.
As for 'u' vs 'U', a 'U' is emitted in the .h file even if I put a 'u'
in the .scm.
If 'u' in the output is preferred, maybe someone can point out the
reason for this?
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 6:52 ` Stefan Kristiansson
@ 2014-05-22 7:10 ` Stefan Kristiansson
2014-05-22 13:33 ` Pierre Muller
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Stefan Kristiansson @ 2014-05-22 7:10 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller
On Thu, May 22, 2014 at 09:52:50AM +0300, Stefan Kristiansson wrote:
>
> As for 'u' vs 'U', a 'U' is emitted in the .h file even if I put a 'u'
> in the .scm.
> If 'u' in the output is preferred, maybe someone can point out the
> reason for this?
>
I found the reason, there's a 'string-upcase' a couple of rows above
what the patch touches.
I'm not sure getting around that is worth pursuing though?
Anyway, below is the updated patch.
2014-05-22 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
* enum.scm (gen-enum-decl): Emit 'U' after constants larger than
#x80000000
Index: cgen/enum.scm
===================================================================
RCS file: /cvs/src/src/cgen/enum.scm,v
retrieving revision 1.13
diff -u -r1.13 enum.scm
--- cgen/enum.scm 13 Feb 2010 03:39:15 -0000 1.13
+++ cgen/enum.scm 22 May 2014 07:09:00 -0000
@@ -298,7 +298,10 @@
""
(string-append " = "
(if (number? (cadr e))
- (number->string (cadr e))
+ (string-append (number->string (cadr e))
+ (if (> (cadr e)
+ #x80000000)
+ "U" ""))
(cadr e))))
))
(if (and san? include-sanitize-marker?)
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] Make large enum constants unsigned
2014-05-22 7:10 ` Stefan Kristiansson
@ 2014-05-22 13:33 ` Pierre Muller
[not found] ` <5561926758764771034@unknownmsgid>
2014-05-22 16:14 ` Maciej W. Rozycki
2 siblings, 0 replies; 14+ messages in thread
From: Pierre Muller @ 2014-05-22 13:33 UTC (permalink / raw)
To: 'Stefan Kristiansson', 'Maciej W. Rozycki'
Cc: cgen, 'Alan Modra', fche, 'Christian Svensson'
Couldn't we support 64-bit constants already,
by adding
+ (if (> (cadr e)
+ #x100000000)
+ "LL" ""))
Right after the conditional "U" postfix?
This way, the pre C-99 standard would only be violated when
it is really required anyhow, no?
Pierre Muller
> -----Message d'origine-----
> De : Stefan Kristiansson [mailto:stefan.kristiansson@saunalahti.fi]
> Envoyé : jeudi 22 mai 2014 09:10
> À : Maciej W. Rozycki
> Cc : cgen@sourceware.org; Alan Modra; fche@sourceware.org; Christian
> Svensson; Pierre Muller
> Objet : Re: [PATCH] Make large enum constants unsigned
>
> On Thu, May 22, 2014 at 09:52:50AM +0300, Stefan Kristiansson wrote:
> >
> > As for 'u' vs 'U', a 'U' is emitted in the .h file even if I put a
> 'u'
> > in the .scm.
> > If 'u' in the output is preferred, maybe someone can point out the
> > reason for this?
> >
>
> I found the reason, there's a 'string-upcase' a couple of rows above
> what the patch touches.
> I'm not sure getting around that is worth pursuing though?
>
> Anyway, below is the updated patch.
>
> 2014-05-22 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>
> * enum.scm (gen-enum-decl): Emit 'U' after constants larger than
> #x80000000
>
> Index: cgen/enum.scm
> ===================================================================
> RCS file: /cvs/src/src/cgen/enum.scm,v
> retrieving revision 1.13
> diff -u -r1.13 enum.scm
> --- cgen/enum.scm 13 Feb 2010 03:39:15 -0000 1.13
> +++ cgen/enum.scm 22 May 2014 07:09:00 -0000
> @@ -298,7 +298,10 @@
> ""
> (string-append " = "
> (if (number? (cadr e))
> - (number->string (cadr e))
> + (string-append (number->string (cadr
e))
> + (if (> (cadr e)
> + #x80000000)
> + "U" ""))
> (cadr e))))
> ))
> (if (and san? include-sanitize-marker?)
^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <5561926758764771034@unknownmsgid>]
* Re: [PATCH] Make large enum constants unsigned
[not found] ` <5561926758764771034@unknownmsgid>
@ 2014-05-22 14:10 ` Stefan Kristiansson
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Kristiansson @ 2014-05-22 14:10 UTC (permalink / raw)
To: Pierre Muller
Cc: Stefan Kristiansson, Maciej W. Rozycki, cgen, Alan Modra, fche,
Christian Svensson
On Thu, May 22, 2014 at 4:33 PM, Pierre Muller
<pierre.muller@ics-cnrs.unistra.fr> wrote:
> Couldn't we support 64-bit constants already,
> by adding
> + (if (> (cadr e)
> + #x100000000)
> + "LL" ""))
> Right after the conditional "U" postfix?
>
> This way, the pre C-99 standard would only be violated when
> it is really required anyhow, no?
>
That would solve the standard violation problem, but the fact that
enums are limited to the size of an int still remains, so it wouldn't
help much.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 7:10 ` Stefan Kristiansson
2014-05-22 13:33 ` Pierre Muller
[not found] ` <5561926758764771034@unknownmsgid>
@ 2014-05-22 16:14 ` Maciej W. Rozycki
2 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2014-05-22 16:14 UTC (permalink / raw)
To: Stefan Kristiansson
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller
On Thu, 22 May 2014, Stefan Kristiansson wrote:
> I found the reason, there's a 'string-upcase' a couple of rows above
> what the patch touches.
> I'm not sure getting around that is worth pursuing though?
Not for me.
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 5:30 [PATCH] Make large enum constants unsigned Stefan Kristiansson
2014-05-22 6:22 ` Maciej W. Rozycki
@ 2014-05-22 15:11 ` Frank Ch. Eigler
2014-05-22 17:34 ` Stefan Kristiansson
1 sibling, 1 reply; 14+ messages in thread
From: Frank Ch. Eigler @ 2014-05-22 15:11 UTC (permalink / raw)
To: Stefan Kristiansson
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller,
Maciej W. Rozycki
Hi -
> this fixes a bug noted in two threads on the binutils mailing list:
> https://sourceware.org/ml/binutils/2014-05/msg00152.html
> https://sourceware.org/ml/binutils/2014-05/msg00195.html
>
> Both threads describe the problem pretty well, but the gist of it
> is that constants are generated that will be interpreted as signed.
>
> 2014-05-22 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>
> * enum.scm (gen-enum-decl): Emit 'ULL' after constants larger than
> #x80000000
> [...]
> - (number->string (cadr e))
> + (string-append (number->string (cadr e))
> + (if (> (cadr e)
> + #x80000000)
> + "ULL" ""))
Have you thought of having cgen emit it as
(string-append "(signed int)" (number->string (cadr e)))
... or even find a number->string conversion function that takes into
account the destionation language's encoding constraints.
- FChE
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 15:11 ` Frank Ch. Eigler
@ 2014-05-22 17:34 ` Stefan Kristiansson
2014-05-22 21:06 ` Frank Ch. Eigler
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Kristiansson @ 2014-05-22 17:34 UTC (permalink / raw)
To: Frank Ch. Eigler
Cc: Stefan Kristiansson, cgen, Alan Modra, fche, Christian Svensson,
Pierre Muller, Maciej W. Rozycki
On Thu, May 22, 2014 at 6:09 PM, Frank Ch. Eigler <fche@elastic.org> wrote:
> Have you thought of having cgen emit it as
>
> (string-append "(signed int)" (number->string (cadr e)))
>
Ignoring the 'string-upcase' that would turn that into "(SIGNED INT)",
how would that be better than?
(string-append (number->string (cadr e)) "U")
I dismissed that, since it would create a lot larger diff in generated
files than in the isolated cases where it's actually a problem.
> ... or even find a number->string conversion function that takes into
> account the destionation language's encoding constraints.
>
That might certainly be an option, but way over my current Scheme capabilities.
If you give me some pointers I could try to improve them.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 17:34 ` Stefan Kristiansson
@ 2014-05-22 21:06 ` Frank Ch. Eigler
2014-05-22 21:42 ` Maciej W. Rozycki
2014-05-29 20:09 ` Stefan Kristiansson
0 siblings, 2 replies; 14+ messages in thread
From: Frank Ch. Eigler @ 2014-05-22 21:06 UTC (permalink / raw)
To: Stefan Kristiansson
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller,
Maciej W. Rozycki
Hi -
> > (string-append "(signed int)" (number->string (cadr e)))
> [...]
> how would that be better than?
> (string-append (number->string (cadr e)) "U")
It would be a closer match to expressing our desire to match C enum
typing, but I'd be fine with "U" suffixing too.
> I dismissed that, since it would create a lot larger diff in generated
> files than in the isolated cases where it's actually a problem. [...]
IMHO, that's not such a big deal. We don't usually look closely at
the diffs of generated files, once the generating tools are trustworthy.
- FChE
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 21:06 ` Frank Ch. Eigler
@ 2014-05-22 21:42 ` Maciej W. Rozycki
2014-05-22 22:20 ` Frank Ch. Eigler
2014-05-29 20:09 ` Stefan Kristiansson
1 sibling, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2014-05-22 21:42 UTC (permalink / raw)
To: Frank Ch. Eigler
Cc: Stefan Kristiansson, cgen, Alan Modra, fche, Christian Svensson,
Pierre Muller
On Thu, 22 May 2014, Frank Ch. Eigler wrote:
> > > (string-append "(signed int)" (number->string (cadr e)))
> > [...]
> > how would that be better than?
> > (string-append (number->string (cadr e)) "U")
>
> It would be a closer match to expressing our desire to match C enum
> typing, but I'd be fine with "U" suffixing too.
TBH I don't understand what you mean here, a C compiler is free to choose
from `char', `int' and `unsigned int' as the underlying (compatible) type
of an enumeration as long as the type can represent all the member values
defined. How exactly the signed `int' type is going to be advantageous
over the `unsigned int' type here? If this is used to produce a bit mask,
which I gather it is, then I find using an unsigned type more natural.
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 21:42 ` Maciej W. Rozycki
@ 2014-05-22 22:20 ` Frank Ch. Eigler
0 siblings, 0 replies; 14+ messages in thread
From: Frank Ch. Eigler @ 2014-05-22 22:20 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Stefan Kristiansson, cgen, Alan Modra, fche, Christian Svensson,
Pierre Muller
Hi -
> > > > (string-append "(signed int)" (number->string (cadr e)))
> > > [...]
> > > how would that be better than?
> > > (string-append (number->string (cadr e)) "U")
> >
> > It would be a closer match to expressing our desire to match C enum
> > typing, but I'd be fine with "U" suffixing too.
>
> TBH I don't understand what you mean here, a C compiler is free to choose
> from `char', `int' and `unsigned int' as the underlying (compatible) type
> of an enumeration as long as the type can represent all the member values
> defined. [...]
I'm not a language lawyer, and in different levels of the language
this has changed, but my understanding is that to be most compatible,
the enumerators (the given literals) need to be 'int's. (It's a
separate question as to what type the compiler would allocate to an
enum FOO variable.)
> over the `unsigned int' type here? If this is used to produce a bit mask,
> which I gather it is, then I find using an unsigned type more natural.
I see what you mean.
- FChE
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-22 21:06 ` Frank Ch. Eigler
2014-05-22 21:42 ` Maciej W. Rozycki
@ 2014-05-29 20:09 ` Stefan Kristiansson
2014-05-29 20:32 ` Frank Ch. Eigler
1 sibling, 1 reply; 14+ messages in thread
From: Stefan Kristiansson @ 2014-05-29 20:09 UTC (permalink / raw)
To: Frank Ch. Eigler
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller,
Maciej W. Rozycki
On Thu, May 22, 2014 at 05:06:21PM -0400, Frank Ch. Eigler wrote:
> Hi -
>
> > > (string-append "(signed int)" (number->string (cadr e)))
> > [...]
> > how would that be better than?
> > (string-append (number->string (cadr e)) "U")
>
> It would be a closer match to expressing our desire to match C enum
> typing, but I'd be fine with "U" suffixing too.
>
>
> > I dismissed that, since it would create a lot larger diff in generated
> > files than in the isolated cases where it's actually a problem. [...]
>
> IMHO, that's not such a big deal. We don't usually look closely at
> the diffs of generated files, once the generating tools are trustworthy.
>
Ok, that's fine by me if others are ok with it.
Below is the patch that does that.
2014-05-29 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
* enum.scm (gen-enum-decl): Emit 'U' after constants.
Index: enum.scm
===================================================================
RCS file: /cvs/src/src/cgen/enum.scm,v
retrieving revision 1.13
diff -u -r1.13 enum.scm
--- enum.scm 13 Feb 2010 03:39:15 -0000 1.13
+++ enum.scm 29 May 2014 20:08:12 -0000
@@ -298,7 +298,8 @@
""
(string-append " = "
(if (number? (cadr e))
- (number->string (cadr e))
+ (string-append (number->string (cadr e))
+ "U")
(cadr e))))
))
(if (and san? include-sanitize-marker?)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Make large enum constants unsigned
2014-05-29 20:09 ` Stefan Kristiansson
@ 2014-05-29 20:32 ` Frank Ch. Eigler
0 siblings, 0 replies; 14+ messages in thread
From: Frank Ch. Eigler @ 2014-05-29 20:32 UTC (permalink / raw)
To: Stefan Kristiansson
Cc: cgen, Alan Modra, fche, Christian Svensson, Pierre Muller,
Maciej W. Rozycki
Hi -
> [...]
> Ok, that's fine by me if others are ok with it.
> Below is the patch that does that.
>
> 2014-05-29 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>
> * enum.scm (gen-enum-decl): Emit 'U' after constants.
> [...]
One last piece of homework: can you identify in your patch the range
of c compilers / standards-compliance-CFLAGS tested with this change?
I'm a bit worried that U-suffixing may be rejected by earlier or
non-gnu compilers.
A little experiment shows, gcc 4.8 with -ansi -pedantic outright
rejects large integers, with or without U suffix.
gcc -ansi -pedantic -c foo.c
foo.c:1:14: warning: ISO C restricts enumerator values to range of âintâ [-Wpedantic]
enum i { x = 4000000000U };
It accepts with a (int)4000000000 casting formulation instead.
- FChE
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-05-29 20:32 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-22 5:30 [PATCH] Make large enum constants unsigned Stefan Kristiansson
2014-05-22 6:22 ` Maciej W. Rozycki
2014-05-22 6:52 ` Stefan Kristiansson
2014-05-22 7:10 ` Stefan Kristiansson
2014-05-22 13:33 ` Pierre Muller
[not found] ` <5561926758764771034@unknownmsgid>
2014-05-22 14:10 ` Stefan Kristiansson
2014-05-22 16:14 ` Maciej W. Rozycki
2014-05-22 15:11 ` Frank Ch. Eigler
2014-05-22 17:34 ` Stefan Kristiansson
2014-05-22 21:06 ` Frank Ch. Eigler
2014-05-22 21:42 ` Maciej W. Rozycki
2014-05-22 22:20 ` Frank Ch. Eigler
2014-05-29 20:09 ` Stefan Kristiansson
2014-05-29 20:32 ` Frank Ch. Eigler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).