pnkfelix / larceny-test

test import of trac db
Other
2 stars 0 forks source link

macro-expander bizarro #45

Closed larceny-trac-import closed 11 years ago

larceny-trac-import commented 11 years ago

_Reported by: will on Thu Apr 13 14:44:07 2006 _ The (macro-expander) parameter behaves differently in bootstrap heaps (e.g. sparc.heap), compiler development heaps (e.g. twobit.heap), and production heaps (e.g. larceny.heap). The differences can be seen by executing the following code:

; Using twobit:

> (macro-expander)
#<PROCEDURE>

> ((macro-expander) '(begin 3 4) (interaction-environment))
((lambda ()
   (begin)
   '(() () () () #f)
   (begin '3 '4)))

> (macro-expand '(begin 3 4) (interaction-environment))
((lambda ()
   (begin)
   '(() () () () #f)
   ((begin begin) '3 '4)))

> (macro-expand '(begin 3 4)
                (environment-syntax-environment (interaction-environment)))
BUG in macro expander: 
Bug detected in m-expand
special
(begin 3 4)
'#f

The behavior seen in twobit.heap runs much faster than in the other heaps, so this is a performance issue as well.

larceny-trac-import commented 11 years ago

Author: pnkfelix I don't know if this is relevant or not, but I spent some time tonight experimenting with petit.bin, petit-twobit.bin, and petit-larceny.bin, along with associated heaps (getting petit-larceny.heap required jumping some hurdles).

And in the process, I discovered this fun thing in Util/petit-larceny-heap.sch:

      (install-procedures (interaction-environment)
                          '(; Compilation
                compile-files
                            ;macro-expand-expression
                            ...

(note the commented out procedure), but down below:

;;; Redefine MACRO-EXPAND in the interaction environment.

(define macro-expand
  (lambda (expr . rest)
    (let ((env (if (null? rest)
                   (interaction-environment)
                   (car rest))))
      (macro-expand-expression expr env))))

I don't know if its okay to refer to macro-expand-expression like that without having it installed above. Certainly when I tried to run macro-expand (under % petit-larceny.bin -heap petit-larceny.heap), I got the following error:

> (macro-expand '(begin 3 4) (interaction-environment))

Error: Reference to undefined global variable "macro-expand-expression".

Oh, and also, the procedure-expression procedure may be useful at times; it gave nice output for (procedure-expression (macro-expander)).

larceny-trac-import commented 11 years ago

Author: pnkfelix More notes (mostly for myself):

Our heaps are (usually) generated by the following files in Util/:

FYI:

larceny-trac-import commented 11 years ago

Author: pnkfelix Another note, though it may be a bit orthogonal to the issues of this particular bug:

Compiler/driver-larceny.sch's compile-file uses the syntax environment of the currently active interaction environment to expand macros that it encounters.

Compiler/driver-twobit.sch's compile-file '''always''' uses the-usual-syntactic-environment (or rather, a copy of it).

This seems to be insufficiently expressive.

At the very least, I would have expected us to offer a nice interface for compiling a file using a syntactic environment provided by the client; supporting the semantics of something like Flatt's "You Want It When" seems like you need that sort of flexibility. (Well, apart from the fact that Flatt's system would have the files themselves indicate which modules they are requiring, so perhaps it becomes an orthogonal issue at that point. But I can imagne variants on "You Want It When" where you specify the syntax environment dependencies in a file separate from the file you are compiling...)

Maybe the intention is that you always setup your environment as you want it before compiling a file. Whatever the intention is, I suspect we really want to offer a richer set of primitives here.

larceny-trac-import commented 11 years ago

Author: pnkfelix More notes (running ./twobit on a Sparc native build)

> (define sse standard-syntactic-environment)
sse

> (define ese (environment-syntax-environment (interaction-environment)))
ese

> (eq? (syntactic-lookup sse 'begin) denotation-of-begin)
#t

> (eq? (syntactic-lookup ese 'begin) denotation-of-begin)
#f

That seems likely to be wrong.

larceny-trac-import commented 11 years ago

Author: pnkfelix Another comment:

In October of 2000, Lars introduced changeset:1584, which was a rewrite of Util/std-heap.sch (now named Util/larceny-heap.sch). In that rewrite, Lars first introduced the strangeness of redefining macro-expand to consume a full-fledged runtime environment as input (rather than just a syntactic environment such as (environment-syntax-environment (interaction-environment)).

So this change has been in place for quite a long time, despite the fact that this interface for macro-expand is totally inconsistent with the interface to macro-expand}} offered by the twobit heap (which is defined inCompiler/expand.sch``` I believe).

We might want to try to make the two interfaces consistent. My inclination would be to make macro-expand always take a syntactic environment; however, some of Joe Marshall's work in Lib/MzScheme seems to indicate that he found the need to include "auxilliary information" in the runtime environment that was used by his port of VanTondersSimpleMacros. So that might lead us to make the interface take a runtime environment just to have access to that auxilliary information if necessary...

larceny-trac-import commented 11 years ago

Author: pnkfelix Actually Lars introduced the strangeness about macro-expand's interface much earlier, in December of 1998, with changeset:303. It was just in a different file so I didn't notice it until I explored the repository a bit more last night.

And that means that it wasn't even strange, because the macro-expand procedure defined in source:/trunk/larceny_src/Compiler/expand.sch#303 in that time period didn't take a second argument. So it seems that it was Will who violated the precedent set by Lars, rather than the other way around.

larceny-trac-import commented 11 years ago

Author: pnkfelix Okay, I'm narrowing in what's going on.

I'm pretty sure that the reason that (macro-expander) is going faster is because it's got a dynamic-wind where it (temporarily) does (compiler-switches 'standard). This makes macro expansion much faster; it probably removes some expansion-time optimizations that we are doing with the default switches???

[[ However, I have not managed to verify that Twobit is actually acting in this manner; it is only a hunch at this point. ]]

I have managed to get Larceny to act more like Twobit by inserting code of the following form into Util/larceny-heap.sch, mirroring some work that Lars did in Util/petit-larceny-heap.sch that probably should have been applied to both heap generating files.

  ; Install twobit's macro expander as the interpreter's ditto

  (macro-expander (lambda (form environment)
            (let ((switches (compiler-switches 'get)))
              (dynamic-wind
              (lambda ()
                (compiler-switches 'standard))
              (lambda ()
                (macro-expand form 
                               (environment-syntax-environment environment)))
              (lambda ()
                (compiler-switches 'set! switches))))))
  ; Kids, don't try this at home
  (vector-like-set! (interaction-environment)
            4
            (the-usual-syntactic-environment))

Note that if you do the above, you can use the same (macro-expander) call to do expansion under both Twobit and Larceny. If you get rid of the dynamic-wind stuff in the above, calls to (macro-expander) become many times slower.

larceny-trac-import commented 11 years ago

Author: pnkfelix Ticket #55 explains why we saw different behavior in the first and third examples provided by Will. (The second example is just a bogus misuse of the macro-expand interface.)

In particular,

((macro-expander) '(begin 3 4) (interaction-environment))

is getting the macro expansion routine that was sealed under Larceny when it was built, and therefore that works fine with the syntactic environment attached to (interaction-environment).

On the other hand,

(macro-expand '(begin 3 4) 
   (environment-syntax-environment (interaction-environment)))

is attempting to use the macro-expand procedure of the ''currently loaded'' Twobit (which has its own notion of what, for example, the denotation-of-begin is) with a syntactic environment that has an incompatible set of bindings.

This doesn't directly explain why Larceny's macro expander is behaving badly w.r.t. performance, but I think that my last few comments on the setting of the macro-expander parameter and on how the compiler-switches may be modified when you go into expansion might provide insight there.

larceny-trac-import commented 11 years ago

Author: pnkfelix So after discussing the matter with Will, we've decided to fix the problem with the following changes:

  1. Set up the macro-expander in the larceny heap the same way that it is set up in the petit larceny heap.
    • This will fix the inconsistency between how Twobit and Larceny behave when you use the ((macro-expander) ...) approach to expansion.
  2. Alpha rename the macro-expand function defined by Twobit (in Compiler/expand.sch to something like twobit-expand.
    • This will fix the inconsistent handling of what sort of environment the macro-expand function expects, because now we'll have two functions with different names because they have different interfaces for how they are used:
    • twobit-expand will expect to receive a syntactic environment as its second argument (thus preserving Twobit's independence from how the underlying Scheme implementation chooses to represent the environments returned by procedures like interaction-environment)
    • macro-expand, which will be defined by Larceny itself, will expect to receive a Larceny runtime-environment (that it will subsequently pull the syntactic environment out of).

These changes alone don't directly address the performance issue of the two setups. But at least then we'll have a consistent interface upon which we can test hypotheses about the source of the performance differential between Twobit and Larceny; I'll open up a new ticket for such investigation after I do the above tasks.

These changes also don't address the issues raised in Ticket #55; we'll delay addressing that for later.

larceny-trac-import commented 11 years ago

Author: pnkfelix (see changeset:2869 for the fix I incorporated.)