* Type-error in C code
@ 2010-11-12 14:02 Stefan Monnier
2010-11-12 14:21 ` Julien Danjou
2010-11-12 18:45 ` Andreas Schwab
0 siblings, 2 replies; 35+ messages in thread
From: Stefan Monnier @ 2010-11-12 14:02 UTC (permalink / raw
To: Julien Danjou; +Cc: Jan Djärv, emacs-devel
The following code installed by the recent atom-change has a type-error:
static void
set_wm_state (Lisp_Object frame, int add, Atom atom, Atom value)
{
struct x_display_info *dpyinfo = FRAME_X_DISPLAY_INFO (XFRAME (frame));
x_send_client_event (frame, make_number (0), frame,
dpyinfo->Xatom_net_wm_state,
make_number (32),
/* 1 = add, 0 = remove */
Fcons
(make_number (add ? 1 : 0),
Fcons
(atom,
value != 0 ? value : Qnil)));
}
The error is to put an "Atom" into a cons cell: those can only hold
Lisp_Objects. The usual compilation flags won't catch the error because
both types are actually some kind of integer, but if you
compile --enable-use-lisp-union-type, the C compiler will
dutyfully burp.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 14:02 Type-error in C code Stefan Monnier
@ 2010-11-12 14:21 ` Julien Danjou
2010-11-12 15:12 ` Jan Djärv
2010-11-12 18:45 ` Andreas Schwab
1 sibling, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-12 14:21 UTC (permalink / raw
To: Stefan Monnier; +Cc: Jan Djärv, emacs-devel
On Fri, Nov 12 2010, Stefan Monnier wrote:
> static void
> set_wm_state (Lisp_Object frame, int add, Atom atom, Atom value)
> {
> struct x_display_info *dpyinfo = FRAME_X_DISPLAY_INFO (XFRAME (frame));
>
> x_send_client_event (frame, make_number (0), frame,
> dpyinfo->Xatom_net_wm_state,
> make_number (32),
> /* 1 = add, 0 = remove */
> Fcons
> (make_number (add ? 1 : 0),
> Fcons
> (atom,
> value != 0 ? value : Qnil)));
> }
>
> The error is to put an "Atom" into a cons cell: those can only hold
> Lisp_Objects. The usual compilation flags won't catch the error because
> both types are actually some kind of integer, but if you
> compile --enable-use-lisp-union-type, the C compiler will
> dutyfully burp.
Good catch Stefan. I should use this flag to compile my code now.
The fix should be easy I think, you just need to replace atom with
make_number (atom).
I can provide a patch is that handier.
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 14:21 ` Julien Danjou
@ 2010-11-12 15:12 ` Jan Djärv
2010-11-12 15:22 ` Julien Danjou
2010-11-12 21:15 ` Thien-Thi Nguyen
0 siblings, 2 replies; 35+ messages in thread
From: Jan Djärv @ 2010-11-12 15:12 UTC (permalink / raw
To: Stefan Monnier, emacs-devel
An Atom is 32 bits. make_number can't cover that range when long is 32 bits,
so it is an unsafe thing to do. Best is to revert and go back to using a string.
Jan D.
Julien Danjou skrev 2010-11-12 15.21:
> On Fri, Nov 12 2010, Stefan Monnier wrote:
>
>> static void
>> set_wm_state (Lisp_Object frame, int add, Atom atom, Atom value)
>> {
>> struct x_display_info *dpyinfo = FRAME_X_DISPLAY_INFO (XFRAME (frame));
>>
>> x_send_client_event (frame, make_number (0), frame,
>> dpyinfo->Xatom_net_wm_state,
>> make_number (32),
>> /* 1 = add, 0 = remove */
>> Fcons
>> (make_number (add ? 1 : 0),
>> Fcons
>> (atom,
>> value != 0 ? value : Qnil)));
>> }
>>
>> The error is to put an "Atom" into a cons cell: those can only hold
>> Lisp_Objects. The usual compilation flags won't catch the error because
>> both types are actually some kind of integer, but if you
>> compile --enable-use-lisp-union-type, the C compiler will
>> dutyfully burp.
>
> Good catch Stefan. I should use this flag to compile my code now.
>
> The fix should be easy I think, you just need to replace atom with
> make_number (atom).
>
> I can provide a patch is that handier.
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:12 ` Jan Djärv
@ 2010-11-12 15:22 ` Julien Danjou
2010-11-12 15:28 ` Julien Danjou
` (2 more replies)
2010-11-12 21:15 ` Thien-Thi Nguyen
1 sibling, 3 replies; 35+ messages in thread
From: Julien Danjou @ 2010-11-12 15:22 UTC (permalink / raw
To: Jan Djärv; +Cc: Stefan Monnier, emacs-devel
On Fri, Nov 12 2010, Jan Djärv wrote:
> An Atom is 32 bits. make_number can't cover that range when long is 32
> bits, so it is an unsafe thing to do. Best is to revert and go back to
> using a string.
There's no really no way to make a 32 bits number in elisp?
I mean, I trust you, I am not familiar with elisp's internal, but that
sounds weird.
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:22 ` Julien Danjou
@ 2010-11-12 15:28 ` Julien Danjou
2010-11-12 15:32 ` Julien Danjou
2010-11-12 15:48 ` Eli Zaretskii
2010-11-12 17:13 ` Jan D.
2 siblings, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-12 15:28 UTC (permalink / raw
To: Jan Djärv; +Cc: Stefan Monnier, emacs-devel
On Fri, Nov 12 2010, Julien Danjou wrote:
> There's no really no way to make a 32 bits number in elisp?
> I mean, I trust you, I am not familiar with elisp's internal, but that
> sounds weird.
Ok, reading the Lisp_Object definition explains why. I've learned
something today. :)
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:28 ` Julien Danjou
@ 2010-11-12 15:32 ` Julien Danjou
2010-11-12 15:48 ` Eli Zaretskii
0 siblings, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-12 15:32 UTC (permalink / raw
To: Jan Djärv; +Cc: Stefan Monnier, emacs-devel
On Fri, Nov 12 2010, Julien Danjou wrote:
> Ok, reading the Lisp_Object definition explains why. I've learned
> something today. :)
I've replying again to myself, but I'm still discovering the internal.
Sorry.
It seems we can store 32 bits unsigned using XSETFASTINT.
Reading x_fill_property_data, I see:
--8<---------------cut here---------------start------------->8---
for (iter = data; CONSP (iter); iter = XCDR (iter))
{
Lisp_Object o = XCAR (iter);
if (INTEGERP (o))
val = (long) XFASTINT (o);
--8<---------------cut here---------------end--------------->8---
So IIUC, we could use XSETFASTINT to store the atom. Am I wrong?
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:32 ` Julien Danjou
@ 2010-11-12 15:48 ` Eli Zaretskii
2010-11-12 15:58 ` Julien Danjou
0 siblings, 1 reply; 35+ messages in thread
From: Eli Zaretskii @ 2010-11-12 15:48 UTC (permalink / raw
To: Julien Danjou; +Cc: jan.h.d, monnier, emacs-devel
> From: Julien Danjou <julien@danjou.info>
> Date: Fri, 12 Nov 2010 16:32:10 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
>
> It seems we can store 32 bits unsigned using XSETFASTINT.
No.
> for (iter = data; CONSP (iter); iter = XCDR (iter))
> {
> Lisp_Object o = XCAR (iter);
>
> if (INTEGERP (o))
> val = (long) XFASTINT (o);
> --8<---------------cut here---------------end--------------->8---
>
> So IIUC, we could use XSETFASTINT to store the atom. Am I wrong?
Yes, you are wrong. You should only use XSETFASTINT if you know the
value will not overflow a Lisp integer.
In a nutshell, nothing can work around the basic limitation that only
integer values smaller or equal to most-positive-fixnum can be put
into a Lisp integer.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:22 ` Julien Danjou
2010-11-12 15:28 ` Julien Danjou
@ 2010-11-12 15:48 ` Eli Zaretskii
2010-11-12 17:13 ` Jan D.
2 siblings, 0 replies; 35+ messages in thread
From: Eli Zaretskii @ 2010-11-12 15:48 UTC (permalink / raw
To: Julien Danjou; +Cc: jan.h.d, monnier, emacs-devel
> From: Julien Danjou <julien@danjou.info>
> Date: Fri, 12 Nov 2010 16:22:07 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
>
> There's no really no way to make a 32 bits number in elisp?
Not as an integer, but you could use a float if an integer isn't large
enough.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:48 ` Eli Zaretskii
@ 2010-11-12 15:58 ` Julien Danjou
2010-11-12 16:06 ` John Yates
0 siblings, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-12 15:58 UTC (permalink / raw
To: Eli Zaretskii; +Cc: jan.h.d, monnier, emacs-devel
On Fri, Nov 12 2010, Eli Zaretskii wrote:
Hi Eli,
> Yes, you are wrong. You should only use XSETFASTINT if you know the
> value will not overflow a Lisp integer.
>
> In a nutshell, nothing can work around the basic limitation that only
> integer values smaller or equal to most-positive-fixnum can be put
> into a Lisp integer.
Ok, I did not know that. So XSETFASTINT is really to be fast, not to be
bigger. Thanks for the explanation.
OTOH, x_fill_property_data deals with floats too, so using a float here
sounds like a good solution, as you suggested.
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:58 ` Julien Danjou
@ 2010-11-12 16:06 ` John Yates
2010-11-12 16:20 ` Eli Zaretskii
2010-11-12 19:44 ` Miles Bader
0 siblings, 2 replies; 35+ messages in thread
From: John Yates @ 2010-11-12 16:06 UTC (permalink / raw
To: Eli Zaretskii, jan.h.d, monnier, emacs-devel
On Fri, Nov 12, 2010 at 10:58 AM, Julien Danjou <julien@danjou.info> wrote:
> OTOH, x_fill_property_data deals with floats too, so using a float here
> sounds like a good solution, as you suggested.
Alternatively you could check the atom's value and so long as it is in
range you could generate an elisp integer, falling back to a float
only when the atom is too large.
/john
--
John Yates
257 Nashoba Rd
Concord, MA 01742
978 371-4923
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 16:06 ` John Yates
@ 2010-11-12 16:20 ` Eli Zaretskii
2010-11-12 16:47 ` Andreas Schwab
2010-11-12 19:44 ` Miles Bader
1 sibling, 1 reply; 35+ messages in thread
From: Eli Zaretskii @ 2010-11-12 16:20 UTC (permalink / raw
To: John Yates; +Cc: jan.h.d, monnier, emacs-devel
> Date: Fri, 12 Nov 2010 11:06:26 -0500
> From: John Yates <john@yates-sheets.org>
>
> On Fri, Nov 12, 2010 at 10:58 AM, Julien Danjou <julien@danjou.info> wrote:
>
> > OTOH, x_fill_property_data deals with floats too, so using a float here
> > sounds like a good solution, as you suggested.
>
> Alternatively you could check the atom's value and so long as it is in
> range you could generate an elisp integer, falling back to a float
> only when the atom is too large.
That's what I meant, actually (we already do such things in other
places in Emacs sources).
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 16:20 ` Eli Zaretskii
@ 2010-11-12 16:47 ` Andreas Schwab
0 siblings, 0 replies; 35+ messages in thread
From: Andreas Schwab @ 2010-11-12 16:47 UTC (permalink / raw
To: Eli Zaretskii; +Cc: jan.h.d, emacs-devel, monnier, John Yates
Eli Zaretskii <eliz@gnu.org> writes:
>> Date: Fri, 12 Nov 2010 11:06:26 -0500
>> From: John Yates <john@yates-sheets.org>
>>
>> On Fri, Nov 12, 2010 at 10:58 AM, Julien Danjou <julien@danjou.info> wrote:
>>
>> > OTOH, x_fill_property_data deals with floats too, so using a float here
>> > sounds like a good solution, as you suggested.
>>
>> Alternatively you could check the atom's value and so long as it is in
>> range you could generate an elisp integer, falling back to a float
>> only when the atom is too large.
>
> That's what I meant, actually (we already do such things in other
> places in Emacs sources).
See make_fixnum_or_float.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:22 ` Julien Danjou
2010-11-12 15:28 ` Julien Danjou
2010-11-12 15:48 ` Eli Zaretskii
@ 2010-11-12 17:13 ` Jan D.
2 siblings, 0 replies; 35+ messages in thread
From: Jan D. @ 2010-11-12 17:13 UTC (permalink / raw
To: Stefan Monnier, emacs-devel
Julien Danjou skrev 2010-11-12 16:22:
> On Fri, Nov 12 2010, Jan Djärv wrote:
>
>> An Atom is 32 bits. make_number can't cover that range when long is 32
>> bits, so it is an unsafe thing to do. Best is to revert and go back to
>> using a string.
>
> There's no really no way to make a 32 bits number in elisp?
> I mean, I trust you, I am not familiar with elisp's internal, but that
> sounds weird.
>
Either use a 64-bit OS or use a cons with 16 bits each. I'm not sure if
the send_client_message functions support that, you have to check (I'm
not near the source right now).
Jan D.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 14:02 Type-error in C code Stefan Monnier
2010-11-12 14:21 ` Julien Danjou
@ 2010-11-12 18:45 ` Andreas Schwab
2010-11-12 21:00 ` Stefan Monnier
2010-11-13 20:00 ` Julien Danjou
1 sibling, 2 replies; 35+ messages in thread
From: Andreas Schwab @ 2010-11-12 18:45 UTC (permalink / raw
To: Stefan Monnier; +Cc: Julien Danjou, Jan Djärv, emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> The error is to put an "Atom" into a cons cell: those can only hold
> Lisp_Objects. The usual compilation flags won't catch the error because
> both types are actually some kind of integer, but if you
> compile --enable-use-lisp-union-type, the C compiler will
> dutyfully burp.
I think we should remove the union Lisp_Object, and instead define a
struct Lisp_Object { EMACS_INT i; } (reusing the macros of the non-union
type), and make that the default Lisp_Object at least during
development.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 16:06 ` John Yates
2010-11-12 16:20 ` Eli Zaretskii
@ 2010-11-12 19:44 ` Miles Bader
1 sibling, 0 replies; 35+ messages in thread
From: Miles Bader @ 2010-11-12 19:44 UTC (permalink / raw
To: John Yates; +Cc: Eli Zaretskii, jan.h.d, monnier, emacs-devel
John Yates <john@yates-sheets.org> writes:
> Alternatively you could check the atom's value and so long as it is in
> range you could generate an elisp integer, falling back to a float
> only when the atom is too large.
... and presumably on 64-bit systems (increasingly the norm), a 32-bit
quantity would _always_ fit.
-miles
--
Mayonnaise, n. One of the sauces that serve the French in place of a state
religion.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 18:45 ` Andreas Schwab
@ 2010-11-12 21:00 ` Stefan Monnier
2010-11-15 21:11 ` Julien Danjou
2010-11-15 21:19 ` Andreas Schwab
2010-11-13 20:00 ` Julien Danjou
1 sibling, 2 replies; 35+ messages in thread
From: Stefan Monnier @ 2010-11-12 21:00 UTC (permalink / raw
To: Andreas Schwab; +Cc: Julien Danjou, Jan Djärv, emacs-devel
>> The error is to put an "Atom" into a cons cell: those can only hold
>> Lisp_Objects. The usual compilation flags won't catch the error because
>> both types are actually some kind of integer, but if you
>> compile --enable-use-lisp-union-type, the C compiler will
>> dutyfully burp.
> I think we should remove the union Lisp_Object, and instead define a
> struct Lisp_Object { EMACS_INT i; } (reusing the macros of the non-union
> type), and make that the default Lisp_Object at least during
> development.
Not sure what we be the benefit.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 15:12 ` Jan Djärv
2010-11-12 15:22 ` Julien Danjou
@ 2010-11-12 21:15 ` Thien-Thi Nguyen
2010-11-13 19:19 ` Jan Djärv
1 sibling, 1 reply; 35+ messages in thread
From: Thien-Thi Nguyen @ 2010-11-12 21:15 UTC (permalink / raw
To: emacs-devel
() Jan Djärv <jan.h.d@swipnet.se>
() Fri, 12 Nov 2010 16:12:17 +0100
An Atom is 32 bits.
... with the top 3 bits guaranteed to be zero
(X Windows System Protocol, section 3. Common Types).
In this Emacs, ‘most-positive-fixnum’ is 536870911,
i.e., #x1fffffff, i.e., 29 1s.
Coincidence?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 21:15 ` Thien-Thi Nguyen
@ 2010-11-13 19:19 ` Jan Djärv
2010-11-13 19:52 ` Thien-Thi Nguyen
2010-11-13 20:38 ` Johan Bockgård
0 siblings, 2 replies; 35+ messages in thread
From: Jan Djärv @ 2010-11-13 19:19 UTC (permalink / raw
To: Thien-Thi Nguyen; +Cc: emacs-devel
Thien-Thi Nguyen skrev 2010-11-12 22.15:
> () Jan Djärv<jan.h.d@swipnet.se>
> () Fri, 12 Nov 2010 16:12:17 +0100
>
> An Atom is 32 bits.
>
> ... with the top 3 bits guaranteed to be zero
> (X Windows System Protocol, section 3. Common Types).
>
> In this Emacs, ‘most-positive-fixnum’ is 536870911,
> i.e., #x1fffffff, i.e., 29 1s.
>
> Coincidence?
Interesting. Maybe they used those bits as a type indictor at one point.
Looking at the X source now, I can't find any reson for this.
However, I fixed the code to use make_fixnum_or_float anyway. In the future
somebody may come along and wonder why a 32-bit number is put in a Lisp integer.
Jan D.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-13 19:19 ` Jan Djärv
@ 2010-11-13 19:52 ` Thien-Thi Nguyen
2010-11-13 22:23 ` Jan Djärv
2010-11-13 20:38 ` Johan Bockgård
1 sibling, 1 reply; 35+ messages in thread
From: Thien-Thi Nguyen @ 2010-11-13 19:52 UTC (permalink / raw
To: Jan Djärv; +Cc: emacs-devel
() Jan Djärv <jan.h.d@swipnet.se>
() Sat, 13 Nov 2010 20:19:05 +0100
Interesting. Maybe they used those bits as a type indictor at
one point. Looking at the X source now, I can't find any reson
for this.
IIRC, X was originally conceived and implemented on a Lisp system.
The term "Atom", and its 29 bits max-ness are direct consequences.
However, I fixed the code to use make_fixnum_or_float anyway.
In the future somebody may come along and wonder why a 32-bit
number is put in a Lisp integer.
Not if they understand that an Atom is NOT a 32-bit number.
Perhaps if you revert the change and explain the limits of
the type in a nice comment, future programmers will not be
inclined to wonder when they look (only) at the source.
I think routing through ‘make_fixnum_or_float’ is suboptimal.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 18:45 ` Andreas Schwab
2010-11-12 21:00 ` Stefan Monnier
@ 2010-11-13 20:00 ` Julien Danjou
2010-11-15 16:08 ` Stefan Monnier
1 sibling, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-13 20:00 UTC (permalink / raw
To: Andreas Schwab; +Cc: Jan Djärv, Stefan Monnier, emacs-devel
On Fri, Nov 12 2010, Andreas Schwab wrote:
> I think we should remove the union Lisp_Object, and instead define a
> struct Lisp_Object { EMACS_INT i; } (reusing the macros of the non-union
> type), and make that the default Lisp_Object at least during
> development.
I can't think of a good reason not to do that. Is there one?
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-13 19:19 ` Jan Djärv
2010-11-13 19:52 ` Thien-Thi Nguyen
@ 2010-11-13 20:38 ` Johan Bockgård
2010-11-14 10:21 ` Jan Djärv
1 sibling, 1 reply; 35+ messages in thread
From: Johan Bockgård @ 2010-11-13 20:38 UTC (permalink / raw
To: emacs-devel
Jan Djärv <jan.h.d@swipnet.se> writes:
> However, I fixed the code to use make_fixnum_or_float anyway.
> + value != 0
> + ? make_fixnum_or_float (value) : Qnil)));
This part should be
... ? Fcons (..., Qnil) : Qnil
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-13 19:52 ` Thien-Thi Nguyen
@ 2010-11-13 22:23 ` Jan Djärv
0 siblings, 0 replies; 35+ messages in thread
From: Jan Djärv @ 2010-11-13 22:23 UTC (permalink / raw
To: Thien-Thi Nguyen; +Cc: emacs-devel
Thien-Thi Nguyen skrev 2010-11-13 20.52:
> () Jan Djärv<jan.h.d@swipnet.se>
> () Sat, 13 Nov 2010 20:19:05 +0100
> However, I fixed the code to use make_fixnum_or_float anyway.
> In the future somebody may come along and wonder why a 32-bit
> number is put in a Lisp integer.
>
> Not if they understand that an Atom is NOT a 32-bit number.
> Perhaps if you revert the change and explain the limits of
> the type in a nice comment, future programmers will not be
> inclined to wonder when they look (only) at the source.
>
> I think routing through ‘make_fixnum_or_float’ is suboptimal.
This code is only used when changing fullscreen or sticky frame parameter.
And that does not happen often. An extra check that ensures the value is
below max fixnum is too small to be worth to optimize away. If atoms are 29
bits, they will be fixnum anyway.
Jan D.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-13 20:38 ` Johan Bockgård
@ 2010-11-14 10:21 ` Jan Djärv
0 siblings, 0 replies; 35+ messages in thread
From: Jan Djärv @ 2010-11-14 10:21 UTC (permalink / raw
To: Johan Bockgård; +Cc: emacs-devel
Johan Bockgård skrev 2010-11-13 21.38:
> Jan Djärv<jan.h.d@swipnet.se> writes:
>
>> However, I fixed the code to use make_fixnum_or_float anyway.
>
>> + value != 0
>> + ? make_fixnum_or_float (value) : Qnil)));
>
> This part should be
>
> ... ? Fcons (..., Qnil) : Qnil
Indeed, thanks.
Jan D.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-13 20:00 ` Julien Danjou
@ 2010-11-15 16:08 ` Stefan Monnier
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Monnier @ 2010-11-15 16:08 UTC (permalink / raw
To: Andreas Schwab; +Cc: Jan Djärv, emacs-devel
>> I think we should remove the union Lisp_Object, and instead define a
>> struct Lisp_Object { EMACS_INT i; } (reusing the macros of the non-union
>> type), and make that the default Lisp_Object at least during
>> development.
> I can't think of a good reason not to do that. Is there one?
Data of type "struct ..." is often treated differently by compilers than
data of type int/long/... so the resulting code is likely to be
less efficient.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 21:00 ` Stefan Monnier
@ 2010-11-15 21:11 ` Julien Danjou
2010-11-15 21:19 ` Andreas Schwab
1 sibling, 0 replies; 35+ messages in thread
From: Julien Danjou @ 2010-11-15 21:11 UTC (permalink / raw
To: Stefan Monnier; +Cc: Jan Djärv, Andreas Schwab, emacs-devel
On Fri, Nov 12 2010, Stefan Monnier wrote:
> Not sure what we be the benefit.
Avoid programming mistakes?
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-12 21:00 ` Stefan Monnier
2010-11-15 21:11 ` Julien Danjou
@ 2010-11-15 21:19 ` Andreas Schwab
2010-11-16 5:43 ` Stefan Monnier
1 sibling, 1 reply; 35+ messages in thread
From: Andreas Schwab @ 2010-11-15 21:19 UTC (permalink / raw
To: Stefan Monnier; +Cc: Julien Danjou, Jan Djärv, emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> The error is to put an "Atom" into a cons cell: those can only hold
>>> Lisp_Objects. The usual compilation flags won't catch the error because
>>> both types are actually some kind of integer, but if you
>>> compile --enable-use-lisp-union-type, the C compiler will
>>> dutyfully burp.
>
>> I think we should remove the union Lisp_Object, and instead define a
>> struct Lisp_Object { EMACS_INT i; } (reusing the macros of the non-union
>> type), and make that the default Lisp_Object at least during
>> development.
>
> Not sure what we be the benefit.
Let the compiler burp.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-15 21:19 ` Andreas Schwab
@ 2010-11-16 5:43 ` Stefan Monnier
2010-11-16 7:56 ` Julien Danjou
2010-11-16 9:36 ` Andreas Schwab
0 siblings, 2 replies; 35+ messages in thread
From: Stefan Monnier @ 2010-11-16 5:43 UTC (permalink / raw
To: Andreas Schwab; +Cc: Julien Danjou, Jan Djärv, emacs-devel
>> Not sure what we be the benefit.
> Let the compiler burp.
We already have that with use-union-lisp-type.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 5:43 ` Stefan Monnier
@ 2010-11-16 7:56 ` Julien Danjou
2010-11-16 9:05 ` Stephen J. Turnbull
2010-11-16 15:14 ` Stefan Monnier
2010-11-16 9:36 ` Andreas Schwab
1 sibling, 2 replies; 35+ messages in thread
From: Julien Danjou @ 2010-11-16 7:56 UTC (permalink / raw
To: Stefan Monnier; +Cc: Jan Djärv, Andreas Schwab, emacs-devel
On Tue, Nov 16 2010, Stefan Monnier wrote:
> We already have that with use-union-lisp-type.
And I think it should not be an option. It should be the default.
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 7:56 ` Julien Danjou
@ 2010-11-16 9:05 ` Stephen J. Turnbull
2010-11-16 9:28 ` Julien Danjou
2010-11-16 15:14 ` Stefan Monnier
1 sibling, 1 reply; 35+ messages in thread
From: Stephen J. Turnbull @ 2010-11-16 9:05 UTC (permalink / raw
To: Julien Danjou; +Cc: Jan Djärv, Andreas Schwab, Stefan Monnier, emacs-devel
Julien Danjou writes:
> On Tue, Nov 16 2010, Stefan Monnier wrote:
>
> > We already have that with use-union-lisp-type.
>
> And I think it should not be an option. It should be the default.
Historically, GCC has been buggy with union type in XEmacs, at least.
It had problems (which regressed at least once after being fixed) with
structures inside of unions. Microsoft compilers have also had
trouble with it. That was ages ago == GCC 3.3 or so ... we think. So
maybe it's not a relevant consideration now.
It also may make things slower because it restricts the compiler from
doing some code transformations that turn out (according to Martin
Buchholz, anyway) entire safe in the context of Lisp object
manipulations. Also, in XEmacs it conflicts with the optimizations
that require no variable aliasing (there are a few places where casts
are used to fit Lisp objects into XPointers and the like), which in
some microbenchmarks of the XEmacs bytecode interpreter gave a small
speedup. It's not just that warning; the code is actually wrong.
Martin and Ben Wing claimed that GCC was being too aggressive, but the
GCC developers replied reasonably enough that the code they used was
technically permitted by the standard, and fixing union-type for
XEmacs didn't justify the effort needed to add a special case.
Finally, note that a default *is* an option. Do you want to force
union type for all builds, or do you want to make it the default and
leave the option to the user to use the (possibly faster) bit-flicking
implementation?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 9:05 ` Stephen J. Turnbull
@ 2010-11-16 9:28 ` Julien Danjou
2010-11-16 12:11 ` Stephen J. Turnbull
0 siblings, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-16 9:28 UTC (permalink / raw
To: Stephen J. Turnbull
Cc: Jan Djärv, Andreas Schwab, Stefan Monnier, emacs-devel
On Tue, Nov 16 2010, Stephen J. Turnbull wrote:
> Finally, note that a default *is* an option. Do you want to force
> union type for all builds, or do you want to make it the default and
> leave the option to the user to use the (possibly faster) bit-flicking
> implementation?
Since I don't buy your arguments[1], I think it should NOT be an option,
but the implementation itself.
OTOH, I only talked about using a struct including the integer, not
necessarily the whole union type used by use-union-lisp-type.
We already use that kind of type in XCB[2] without any problem.
[1] Which were probably true a long time ago, but you will need to prove
that it does not compile anywhere and is slow to me in 2010. :)
[2] http://xcb.freedesktop.org
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 5:43 ` Stefan Monnier
2010-11-16 7:56 ` Julien Danjou
@ 2010-11-16 9:36 ` Andreas Schwab
1 sibling, 0 replies; 35+ messages in thread
From: Andreas Schwab @ 2010-11-16 9:36 UTC (permalink / raw
To: Stefan Monnier; +Cc: Julien Danjou, Jan Djärv, emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Not sure what we be the benefit.
>> Let the compiler burp.
>
> We already have that with use-union-lisp-type.
It lacks support for USE_2_TAGS_FOR_INTS.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 9:28 ` Julien Danjou
@ 2010-11-16 12:11 ` Stephen J. Turnbull
0 siblings, 0 replies; 35+ messages in thread
From: Stephen J. Turnbull @ 2010-11-16 12:11 UTC (permalink / raw
To: Julien Danjou; +Cc: Jan Djärv, Andreas Schwab, Stefan Monnier, emacs-devel
Julien Danjou writes:
> Since I don't buy your arguments[1],
I'm not selling them, simply mentioning them for those who aren't
familiar with the history of the option (of course, I'm assuming the
history in Emacs is similar to that of XEmacs, but I think that's
reasonable).
> I think it should NOT be an option, but the implementation itself.
>
> OTOH, I only talked about using a struct including the integer, not
> necessarily the whole union type used by use-union-lisp-type.
Well, please be careful, then; the message you replied to (and quoted)
specifically mentioned use-union-lisp-type. Just because you
specifically described the trivial "int in a struct" type in an
earlier message doesn't mean that you wouldn't advocate making union
Lisp type the default, or perhaps only, implementation of Lisp
objects. That was certainly the way I interpreted it.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 7:56 ` Julien Danjou
2010-11-16 9:05 ` Stephen J. Turnbull
@ 2010-11-16 15:14 ` Stefan Monnier
2010-11-16 15:38 ` Julien Danjou
1 sibling, 1 reply; 35+ messages in thread
From: Stefan Monnier @ 2010-11-16 15:14 UTC (permalink / raw
To: Andreas Schwab; +Cc: Jan Djärv, emacs-devel
>> We already have that with use-union-lisp-type.
> And I think it should not be an option. It should be the default.
Same problem as the structs type: it's not optimized as well as the
int/long... types.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 15:14 ` Stefan Monnier
@ 2010-11-16 15:38 ` Julien Danjou
2010-11-16 17:05 ` Stefan Monnier
0 siblings, 1 reply; 35+ messages in thread
From: Julien Danjou @ 2010-11-16 15:38 UTC (permalink / raw
To: Stefan Monnier; +Cc: Jan Djärv, Andreas Schwab, emacs-devel
On Tue, Nov 16 2010, Stefan Monnier wrote:
> Same problem as the structs type: it's not optimized as well as the
> int/long... types.
May I ask any references about that?
Not that I don't trust you, but I'd like to know why.
--
Julien Danjou
// ᐰ <julien@danjou.info> http://julien.danjou.info
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Type-error in C code
2010-11-16 15:38 ` Julien Danjou
@ 2010-11-16 17:05 ` Stefan Monnier
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Monnier @ 2010-11-16 17:05 UTC (permalink / raw
To: Julien Danjou; +Cc: emacs-devel
>> Same problem as the structs type: it's not optimized as well as the
>> int/long... types.
> May I ask any references about that?
> Not that I don't trust you, but I'd like to know why.
IIRC calling conventions usually dictate that non-scalar types get
passed via copies on the stack rather than in registers.
Also of course, in general structs can't be handled in the same way as
int/long/... by the compiler, so in order to handle single-slot structs
efficiently, the compiler needs to special case these. I'd expect
a mature C compiler to go through the trouble to try and special-case
them, but that's no guarantee that it's going to be as efficient as just
scalar types.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2010-11-16 17:05 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-12 14:02 Type-error in C code Stefan Monnier
2010-11-12 14:21 ` Julien Danjou
2010-11-12 15:12 ` Jan Djärv
2010-11-12 15:22 ` Julien Danjou
2010-11-12 15:28 ` Julien Danjou
2010-11-12 15:32 ` Julien Danjou
2010-11-12 15:48 ` Eli Zaretskii
2010-11-12 15:58 ` Julien Danjou
2010-11-12 16:06 ` John Yates
2010-11-12 16:20 ` Eli Zaretskii
2010-11-12 16:47 ` Andreas Schwab
2010-11-12 19:44 ` Miles Bader
2010-11-12 15:48 ` Eli Zaretskii
2010-11-12 17:13 ` Jan D.
2010-11-12 21:15 ` Thien-Thi Nguyen
2010-11-13 19:19 ` Jan Djärv
2010-11-13 19:52 ` Thien-Thi Nguyen
2010-11-13 22:23 ` Jan Djärv
2010-11-13 20:38 ` Johan Bockgård
2010-11-14 10:21 ` Jan Djärv
2010-11-12 18:45 ` Andreas Schwab
2010-11-12 21:00 ` Stefan Monnier
2010-11-15 21:11 ` Julien Danjou
2010-11-15 21:19 ` Andreas Schwab
2010-11-16 5:43 ` Stefan Monnier
2010-11-16 7:56 ` Julien Danjou
2010-11-16 9:05 ` Stephen J. Turnbull
2010-11-16 9:28 ` Julien Danjou
2010-11-16 12:11 ` Stephen J. Turnbull
2010-11-16 15:14 ` Stefan Monnier
2010-11-16 15:38 ` Julien Danjou
2010-11-16 17:05 ` Stefan Monnier
2010-11-16 9:36 ` Andreas Schwab
2010-11-13 20:00 ` Julien Danjou
2010-11-15 16:08 ` Stefan Monnier
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.