Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-8-23-gea6b18e

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-8-23-gea6b18e

[Ludovic Courtès]

Hello!

(Sorry for the late & light review...)

"Andy Wingo" <wingo@pobox.com> writes:

> diff --git a/libguile/fluids.h b/libguile/fluids.h
> index 7aefd47..0d61fd2 100644
> --- a/libguile/fluids.h
> +++ b/libguile/fluids.h
> @@ -54,13 +54,8 @@
>     grow.
>   */
>  
> -/* The fastest way to acces/modify the value of a fluid.  These macros
> -   do no error checking at all.  The first argument is the index
> -   number of the fluid, obtained via SCM_FLUID_NUM, not the fluid
> -   itself.  You must make sure that the fluid remains protected as
> -   long you use its number since numbers of unused fluids are reused
> -   eventually.
> -*/
> +#define SCM_I_FLUID_P(x)          (!SCM_IMP (x) && SCM_TYP7 (x) == scm_tc7_fluid)

How about renaming it to ‘SCM_FLUID_P’ and making it public?

> +#define SCM_I_FLUID_NUM(x)        ((size_t)SCM_CELL_WORD_1(x))

Could be in #ifdef BUILDING_LIBGUILE?

> diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c
> index 75dd613..5d1e1d6 100644
> --- a/libguile/vm-engine.c
> +++ b/libguile/vm-engine.c
> @@ -47,6 +47,7 @@ VM_NAME (SCM vm, SCM program, SCM *argv, int nargs)
>    SCM *objects = NULL;			/* constant objects */
>    size_t object_count = 0;              /* length of OBJECTS */
>    SCM *stack_limit = vp->stack_limit;	/* stack limit address */
> +  SCM dynstate = SCM_I_CURRENT_THREAD->dynamic_state;

Out of curiosity, what effect does this have on performance?

> +  (<dynref> fluid)
> +  (<dynset> fluid exp)

At first sight ‘fluid-ref’ and ‘fluid-set’ would seem more natural names
to me.  What do you think?

Thanks,
Ludo’.

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-8-23-gea6b18e

[Andy Wingo]

Heya,

On Sun 28 Feb 2010 22:31, ludo@gnu.org (Ludovic Courtès) writes:

> (Sorry for the late & light review...)

Sorry for the late & light reply :)

>> +#define SCM_I_FLUID_P(x)          (!SCM_IMP (x) && SCM_TYP7 (x) == scm_tc7_fluid)
>
> How about renaming it to ‘SCM_FLUID_P’ and making it public?

Done

>> +#define SCM_I_FLUID_NUM(x)        ((size_t)SCM_CELL_WORD_1(x))
>
> Could be in #ifdef BUILDING_LIBGUILE?

Done

>> diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c
>> index 75dd613..5d1e1d6 100644
>> --- a/libguile/vm-engine.c
>> +++ b/libguile/vm-engine.c
>> @@ -47,6 +47,7 @@ VM_NAME (SCM vm, SCM program, SCM *argv, int nargs)
>>    SCM *objects = NULL;			/* constant objects */
>>    size_t object_count = 0;              /* length of OBJECTS */
>>    SCM *stack_limit = vp->stack_limit;	/* stack limit address */
>> +  SCM dynstate = SCM_I_CURRENT_THREAD->dynamic_state;
>
> Out of curiosity, what effect does this have on performance?

Dunno!

>> +  (<dynref> fluid)
>> +  (<dynset> fluid exp)
>
> At first sight ‘fluid-ref’ and ‘fluid-set’ would seem more natural names
> to me.  What do you think?

I had it like that before, but I did it this way to match dynwind and dynlet.

Andy
-- 
http://wingolog.org/

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-8-23-gea6b18e

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> On Sun 28 Feb 2010 22:31, ludo@gnu.org (Ludovic Courtès) writes:

[...]

>>> +  (<dynref> fluid)
>>> +  (<dynset> fluid exp)
>>
>> At first sight ‘fluid-ref’ and ‘fluid-set’ would seem more natural names
>> to me.  What do you think?
>
> I had it like that before, but I did it this way to match dynwind and dynlet.

But it deals with fluids, right?  In what sense does it match dynwind
and dynlet?

Thanks,
Ludo’.

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-8-23-gea6b18e

[Andy Wingo]

Hi,

On Sat 17 Jul 2010 14:52, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> On Sun 28 Feb 2010 22:31, ludo@gnu.org (Ludovic Courtès) writes:
>
> [...]
>
>>>> +  (<dynref> fluid)
>>>> +  (<dynset> fluid exp)
>>>
>>> At first sight ‘fluid-ref’ and ‘fluid-set’ would seem more natural names
>>> to me.  What do you think?
>>
>> I had it like that before, but I did it this way to match dynwind and dynlet.
>
> But it deals with fluids, right?  In what sense does it match dynwind
> and dynlet?

It deals with dynamic extent. dynlet is with-fluids.

Andy
-- 
http://wingolog.org/