Reporting possibly unbound variables

Reporting possibly unbound variables

[Ludovic Courtès]

Hello,

I just committed support for ‘-Wunbound-variable’:

  http://git.sv.gnu.org/cgit/guile.git/commit/?id=f67ddf9dbfec851676806a2f3dff7eae539ac499

It works pretty well, except for occurrences of ‘define-class’ et al.,
which lead to false positives because the code generated by these macros
doesn’t use ‘define’ (this is to make sure a top-level binding is
created, not a lexical one.)

Ideas on how to “fix” it?

(Presumably I won’t work on it again until after 1.9.4.)

Thanks,
Ludo’.

Re: Reporting possibly unbound variables

[Neil Jerram]

ludo@gnu.org (Ludovic Courtès) writes:

> Hello,
>
> I just committed support for ‘-Wunbound-variable’:
>
>   http://git.sv.gnu.org/cgit/guile.git/commit/?id=f67ddf9dbfec851676806a2f3dff7eae539ac499

Looks great!

> It works pretty well, except for occurrences of ‘define-class’ et al.,
> which lead to false positives because the code generated by these macros
> doesn’t use ‘define’ (this is to make sure a top-level binding is
> created, not a lexical one.)
>
> Ideas on how to “fix” it?

Are there deeper problems with compilation and define-class /
module-define?  E.g. is it possible that correct compilation of code
following a define-class or module-define would be dependent on
understanding the define-class/module-define properly?

If so, I guess either the compiler needs to recognise and process these
calls specially - in which case it should be easy for it also to pass
appropriate information to your unbound variable check - or we could
provide some kind of compile-time declaration form, and say that correct
compilation and unbound variable checking requires this to be used.

On a loosely related point, I notice that we have deprecated eval-case
in 1.9.x, in favour of eval-when.  But 1.8.x only has eval-case; so
isn't the deprecation going to make it more difficult to handle code
that has to be different in 1.8.x and 1.9/2.0?

(For example, 1.8.x requires the comma trick in a macro definition that
uses a non-exported procedure from the same module, whereas 1.9/2.0
requires not using a comma.)

Alternatively, should we add a suitable eval-when to the next 1.8.x
release, to help with this?

Regards,
        Neil

Re: Reporting possibly unbound variables

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> Are there deeper problems with compilation and define-class /
> module-define?  E.g. is it possible that correct compilation of code
> following a define-class or module-define would be dependent on
> understanding the define-class/module-define properly?

No.  For any variable that’s not lexically bound, the compiler generates
instructions that amount to ‘(module-ref (current-module) VAR)’.

Note that the warning reports /possibly/ unbound variables.  Variables
that get bound at run-time via ‘module-define!’ or similar are ignored,
and that’s precisely what ‘define-class’ does.

> On a loosely related point, I notice that we have deprecated eval-case
> in 1.9.x, in favour of eval-when.  But 1.8.x only has eval-case; so
> isn't the deprecation going to make it more difficult to handle code
> that has to be different in 1.8.x and 1.9/2.0?

Yes, I agree it’s problematic.  What about leaving both forms for 2.0,
and only deprecating ‘eval-case’ in the next stable series?

> Alternatively, should we add a suitable eval-when to the next 1.8.x
> release, to help with this?

That wouldn’t help much since it wouldn’t work with < 1.8.8.

Thanks,
Ludo’.

Re: Reporting possibly unbound variables

[Andy Wingo]

On Wed 07 Oct 2009 23:53, ludo@gnu.org (Ludovic Courtès) writes:

>> Alternatively, should we add a suitable eval-when to the next 1.8.x
>> release, to help with this?
>
> That wouldn’t help much since it wouldn’t work with < 1.8.8.

You could have a local module in your program that provides eval-when.
Perhaps you could use the new cond-expand key to include it if
necessary.

Andy
-- 
http://wingolog.org/

Re: Reporting possibly unbound variables

[Ludovic Courtès]

Hello!

Andy: What about leaving both ‘eval-case’ and ‘eval-when’ for 2.0, and
only deprecating ‘eval-case’ in the next stable series?

Thanks,
Ludo’.

Re: Reporting possibly unbound variables

[Andy Wingo]

On Tue 20 Oct 2009 10:08, ludo@gnu.org (Ludovic Courtès) writes:

> Andy: What about leaving both ‘eval-case’ and ‘eval-when’ for 2.0, and
> only deprecating ‘eval-case’ in the next stable series?

The problem is that eval-case has really murky semantics -- to me at
least. We've *never* needed to use it in the past, to my knowledge,
except to distinguish toplevel expressions from lexically nested
expressions -- an iffy use, IMO. (Any definition should be valid
anywhere a definition is valid.)

So I think we should /discourage/ eval-case -- but how would people know
if it weren't deprecated? It's still there, in deprecated.scm. I would
say deprecate it now, and remove it in the next series.

Andy
-- 
http://wingolog.org/

eval-case

[Ludovic Courtès]

Hello!

Andy Wingo <wingo@pobox.com> writes:

> The problem is that eval-case has really murky semantics -- to me at
> least. We've *never* needed to use it in the past, to my knowledge,
> except to distinguish toplevel expressions from lexically nested
> expressions -- an iffy use, IMO. (Any definition should be valid
> anywhere a definition is valid.)

Right, it was essentially useless, but supposedly it was added with
Guile-VM in mind---how visionary!  :-)

My suggestion to not deprecate it in 2.0 was motivated by 1.8/2.0
portability.  ‘eval-case’ in 1.8 only understands the ‘load-toplevel’
clause.  In 2.0, we could add a ‘compile’ clause.  That clause is never
true in 1.8, but in 2.0 it could amount to ‘(eval-when (compile) ...)’.
This is so that people could place things like ‘current-reader’ effects
on the right phase:

  (eval-case
    ((compile) (display "current-reader effects for 2.0\n"))
    (else      (display "current-reader effects for 1.8\n")))

(Actually this isn’t perfect because the ‘else’ branch would also been
taken in 2.0 at load-time.)

OTOH, with ‘(cond-expand (guile-2 ...))’, one can achieve this and more,
which may be a good reason to not worry about it.

Thanks,
Ludo’.

Re: eval-case

[Andy Wingo]

On Wed 21 Oct 2009 13:59, ludo@gnu.org (Ludovic Courtès) writes:

> OTOH, with ‘(cond-expand (guile-2 ...))’, one can achieve this and more,
> which may be a good reason to not worry about it.

Would you be happy settling on this?

Andy
-- 
http://wingolog.org/

Re: Reporting possibly unbound variables

[Andy Wingo]

On Wed 07 Oct 2009 22:27, Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> It works pretty well, except for occurrences of ‘define-class’ et al.,
>> which lead to false positives because the code generated by these macros
>> doesn’t use ‘define’ (this is to make sure a top-level binding is
>> created, not a lexical one.)
>>
>> Ideas on how to “fix” it?
>
> Are there deeper problems with compilation and define-class /
> module-define?  E.g. is it possible that correct compilation of code
> following a define-class or module-define would be dependent on
> understanding the define-class/module-define properly?

I don't think there are problems, no...

> If so, I guess either the compiler needs to recognise and process these
> calls specially - in which case it should be easy for it also to pass
> appropriate information to your unbound variable check - or we could
> provide some kind of compile-time declaration form, and say that correct
> compilation and unbound variable checking requires this to be used.

Well the compiler could recognize module-define as a primitive, and
potentially transform it to be a <toplevel-define>. But that would
happen after the unbound-variables check runs -- so I don't know.

Another possibility would be a hack, but hey: have the -Wunbound thing
detect (if (defined? ...) ...) forms, and do something appropriate.

> On a loosely related point, I notice that we have deprecated eval-case
> in 1.9.x, in favour of eval-when.  But 1.8.x only has eval-case; so
> isn't the deprecation going to make it more difficult to handle code
> that has to be different in 1.8.x and 1.9/2.0?

Perhaps, though if you are updating already to 2.0, I would just make a
2.0 compatibility library, as far as possible. In macros you could do
the @/@@ trick.

> (For example, 1.8.x requires the comma trick in a macro definition that
> uses a non-exported procedure from the same module, whereas 1.9/2.0
> requires not using a comma.)

See NEWS line 282 :) And add something or propose new behavior, as
appropriate.

> Alternatively, should we add a suitable eval-when to the next 1.8.x
> release, to help with this?

I think that would be a good idea, yes.

Andy
-- 
http://wingolog.org/

Re: Reporting possibly unbound variables

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> On Wed 07 Oct 2009 22:27, Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> On a loosely related point, I notice that we have deprecated eval-case
>> in 1.9.x, in favour of eval-when.  But 1.8.x only has eval-case; so
>> isn't the deprecation going to make it more difficult to handle code
>> that has to be different in 1.8.x and 1.9/2.0?
>
> Perhaps, though if you are updating already to 2.0, I would just make a
> 2.0 compatibility library, as far as possible.

Yes, you're right.  I think I was a bit confused when I wrote the above,
thinking that eval-case/eval-when was something like cond-expand.  In
fact Ludovic's suggestion for (cond-expand (guile-2) ...) will be
enough.

> In macros you could do the @/@@ trick.
>
>> (For example, 1.8.x requires the comma trick in a macro definition that
>> uses a non-exported procedure from the same module, whereas 1.9/2.0
>> requires not using a comma.)
>
> See NEWS line 282 :)

Nice entry!

> And add something or propose new behavior, as appropriate.

I don't think we need anything else.  People can use @@ if they want
code that works in both 1.8.x and 2.0, or cond-expand for different
versions.

>> Alternatively, should we add a suitable eval-when to the next 1.8.x
>> release, to help with this?
>
> I think that would be a good idea, yes.

As Ludovic said, that would only help 1.8.8 and onwards.  I think it
would be better to add this to a compatibility library, as and when it's
needed.

        Neil