[PATCH] `try-module-autoload' and `current-reader'

[PATCH] `try-module-autoload' and `current-reader'

[Ludovic Courtès]

Hi,

In `boot-9.scm', `try-module-autoload' should be using `load-module',
and not `primitive-load', when loading a non-compiled module.  The
reasons are (i) non-autoloaded modules are loaded using `load-module'
and (ii) `load-module' and `primitive-load' interact differently with
the `current-reader' fluid.

`load-module' uses R4RS `load' which does some framing on the value of
`current-reader', setting it to `#f' (meaning: use the built-in `read')
if no optional reader argument was provided.  OTOH, `primitive-load'
doesn't change the value of `current-reader', thus resulting in a
different behavior.

Thanks,
Ludovic.


2006-01-12  Ludovic Courtès  <ludovic.courtes@laas.fr>

	* boot-9.scm (try-module-autoload): When loading a non-compiled
	file, use `load-module' instead of `primitive-load'.  Doing so
	will result in the use of the build-in reader when loading the
	module, no matter what the current value of `current-reader' is.


--- orig/ice-9/boot-9.scm
+++ mod/ice-9/boot-9.scm
@@ -2180,7 +2180,7 @@
 			    (load-file load-compiled full)))
 		      ((%search-load-path file)
 		       => (lambda (full)
-			    (load-file primitive-load full))))))
+			    (load-file load-module full))))))
 	    (lambda () (set-autoloaded! dir-hint name didit)))
 	   didit))))
 


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Hi,
>
> In `boot-9.scm', `try-module-autoload' should be using `load-module',
> and not `primitive-load', when loading a non-compiled module.  The
> reasons are (i) non-autoloaded modules are loaded using `load-module'
> and (ii) `load-module' and `primitive-load' interact differently with
> the `current-reader' fluid.
>
> `load-module' uses R4RS `load' which does some framing on the value of
> `current-reader', setting it to `#f' (meaning: use the built-in `read')
> if no optional reader argument was provided.  OTOH, `primitive-load'
> doesn't change the value of `current-reader', thus resulting in a
> different behavior.

Hmm; I think just adding (with-fluids ((current-reader #f)) ...)
around the (load-file ...) call would be better.  Otherwise we would
be introducing two extra changes:

1. Starting a new stack (the start-stack form in R4RS's load).  This
   affects backtraces, and it is occasionally useful for a backtrace
   to show that module X is being loaded because of code from module
   Y.

2. Interpreting a file path beginning with "." as relative to the
   current module's load filename.

What do you think?

     Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Ludovic Courtès]

Hi,

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

> ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
>> Hi,
>>
>> In `boot-9.scm', `try-module-autoload' should be using `load-module',
>> and not `primitive-load', when loading a non-compiled module.  The
>> reasons are (i) non-autoloaded modules are loaded using `load-module'
>> and (ii) `load-module' and `primitive-load' interact differently with
>> the `current-reader' fluid.

[...]

> Hmm; I think just adding (with-fluids ((current-reader #f)) ...)
> around the (load-file ...) call would be better.  Otherwise we would
> be introducing two extra changes:
>
> 1. Starting a new stack (the start-stack form in R4RS's load).  This
>    affects backtraces, and it is occasionally useful for a backtrace
>    to show that module X is being loaded because of code from module
>    Y.
>
> 2. Interpreting a file path beginning with "." as relative to the
>    current module's load filename.

Change #1 would just make autoload's behavior equivalent to that of
explicit loading via `use-module'.  It seems important to me that
autoload and `use-module' behave identically, i.e., either they both use
`load-module' and incidentally set up a new stack, or none of them does.

Also, the `with-fluids' framing is already provided by R4RS `load' and
consequently by `load-module' so I think we should avoid duplicating it.

I don't understand your second point.  In `try-module-autoload', the
name of the module _to be loaded_ is passed, and its path is inferred
from that name, not relatively to the current module's path.  Am I
missing something?

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Hi,
>
> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> 1. Starting a new stack (the start-stack form in R4RS's load).  This
>>    affects backtraces, and it is occasionally useful for a backtrace
>>    to show that module X is being loaded because of code from module
>>    Y.
>>
>> 2. Interpreting a file path beginning with "." as relative to the
>>    current module's load filename.
>
> Change #1 would just make autoload's behavior equivalent to that of
> explicit loading via `use-module'.

No, it would change use-module as well.  Please follow through the
code:

process-use-modules -> resolve-interface -> resolve-module ->
try-load-module -> try-module-autoload

(Arguably try-module-autoload could be better named.)

>  It seems important to me that
> autoload and `use-module' behave identically, i.e., either they both use
> `load-module' and incidentally set up a new stack, or none of them does.

Agreed, but try-module-autoload is used for both, so they will behave
the same as each other whatever we do.  The issue is whether they
behave the same as they do before this patch.

> Also, the `with-fluids' framing is already provided by R4RS `load' and
> consequently by `load-module' so I think we should avoid duplicating it.

This tiny amount of duplication doesn't concern me.

> I don't understand your second point.  In `try-module-autoload', the
> name of the module _to be loaded_ is passed, and its path is inferred
> from that name, not relatively to the current module's path.  Am I
> missing something?

Yes, but it's now covered in another subthread, so I won't repeat here.

Regards,
        Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Ludovic Courtès]

Hi,

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

> process-use-modules -> resolve-interface -> resolve-module ->
> try-load-module -> try-module-autoload
>
> (Arguably try-module-autoload could be better named.)

Sorry, I had missed that point.

>> Also, the `with-fluids' framing is already provided by R4RS `load' and
>> consequently by `load-module' so I think we should avoid duplicating it.
>
> This tiny amount of duplication doesn't concern me.

But still, shouldn't we use `load' (not `load-module', because of the
path concern) to load modules?  If so, the patch would just substitute
`load' to `primitive-load'.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> But still, shouldn't we use `load' (not `load-module', because of the
> path concern) to load modules?  If so, the patch would just substitute
> `load' to `primitive-load'.

But that would still bring in a start-stack and so change backtraces,
so I'd prefer not.

I really think with-fluids is the way to go.  If you agree, I'll make
this change in CVS.

     Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Ludovic Courtès]

Hi,

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

> But that would still bring in a start-stack and so change backtraces,
> so I'd prefer not.

Oh, right.

> I really think with-fluids is the way to go.  If you agree, I'll make
> this change in CVS.

Ok, let's go for it!  ;-)

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Hi,
>
> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> But that would still bring in a start-stack and so change backtraces,
>> so I'd prefer not.
>
> Oh, right.
>
>> I really think with-fluids is the way to go.  If you agree, I'll make
>> this change in CVS.
>
> Ok, let's go for it!  ;-)

This is in CVS now.

     Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Kevin Ryde]

Neil Jerram <neil@ossau.uklinux.net> writes:
>
>    it is occasionally useful for a backtrace to show that module X
>    is being loaded because of code from module Y.

Yes, I've wrestled with that on some inadvertently circular autoloads.
I ended up using %load-hook to show what was actually happening
though.

> 2. Interpreting a file path beginning with "." as relative to the
>    current module's load filename.

I've been meaning to add something to the docs on how load is relative
to the current file (unless you use "./").  A carry-over from emacs or
something I suppose.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> I've been meaning to add something to the docs on how load is relative
> to the current file (unless you use "./").

Can you explain that?

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> Can you explain that?

See load-module in boot-9.scm, hairy stuff.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] `try-module-autoload' and `current-reader'

[Neil Jerram]

Kevin Ryde <user42@zip.com.au> writes:

> ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>>
>> Can you explain that?
>
> See load-module in boot-9.scm, hairy stuff.

And if called from try-module-autoload, I believe this hairy stuff
would come into play if %load-path included relative directories (such
as just ".") and a module name was resolved to a file under a relative
%load-path directory.

     Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel