ds26gte / pyret-lang

The Pyret language.
Other
2 stars 0 forks source link

Something's up with the order of parsing struct definitions #17

Closed schanzer closed 8 years ago

schanzer commented 8 years ago

(make-enemy) 42 (define-struct enemy ())

Throws an error about make-enemy not being defined. This error disappears if you: 1) comment out the middle line 2) swap the first and last lines

ds26gte commented 8 years ago

Fixed in ds26gte/code.pyret.org@fed9601.

(Note this doesn't tackle forward references within definitions, which will be handled separately in a different bugfix.)

schanzer commented 8 years ago

Awesome. I'll leave this issue open to track that issue too.

ds26gte commented 8 years ago

Hmm, what exactly is our policy on forward references? Even WeScheme doesn't seem to allow them, although it seems to allow non-function definitions to move ahead of function definitions in the definition window, which feature can be coded. (Pyret doesn't allow forward references at all.)

Would mimicking WeScheme be sufficient?

schanzer commented 8 years ago

Absolutely.

ds26gte commented 8 years ago

Fixed in ds26gte/code.pyret.org@6422ad2.

Summary of fix: In the definitions window, all top-level definitions (included those generated for define-struct) get bubbled up ahead of other expressions, so the latter can refer to the definitions. Furthermore, definitions of non-functions get bubbled higher than those of functions, so the bodies of the latter can refer to the former.

Example: the order (in the definitions window)

(define (fa) x)
(define fb (lambda () x))
(define fc (let ((y 2)) (lambda () (+ x y))))

(define x 42)

will load correctly, despite fa, fb, fc all forward-referencing x.

As a bonus (this won't work in WeScheme!):

(define m (+ n 1))
(define n 9)

will also load correctly, despite the n not being inside a closure. (Of course, these definitions can't be cyclic.)

schanzer commented 8 years ago

FANASTIC

schanzer commented 8 years ago

It looks like there's still an issue with struct definitions not being put first:

This program runs perfectly (and impressively!), but only when I move the define-struct statements to the top.

ds26gte commented 8 years ago

Is a smaller witness file available? Cutting and pasting this one into my setup (Ubuntu, Chrome) causes browser freeze. I tried fashioning smaller files myself, but they all seem to pass: I can't tell what exactly about this file triggers the bug.

schanzer commented 8 years ago

It freezes the browser? Yikes - that may be a bug in and of itself!

Spyret looks like it's being updated right now (application error when I try to load it), but I'll try to reduce the test case as soon as it's back.

schanzer commented 8 years ago

Okay, here's an interesting test case:

(define (f) (make-enemy))
(define g pi)
(define-struct enemy ())

This will compile and run if any of the following are done:

  1. Comment out line 2
  2. Swap lines 2 and 3
  3. Swap lines 1 and 2
  4. Swap lines 1 and 3
  5. Change pi to a value literal.
ds26gte commented 8 years ago

Fixed in ds26gte/code.pyret.org@e8f0d5f. (When bubbling top-level definitions to the top in the final parsed block, ensure that those corresponding to define-struct are the crème de la crème.)

schanzer commented 8 years ago

MAHvelous!

schanzer commented 8 years ago

Not sure if you've pushed this, but now I see the following breaking: (define SCREEN_WIDTH 500) (define SCREEN_HEIGHT 500) (define EMPTY_SCENE (empty-scene SCREEN_WIDTH SCREEN_HEIGHT))

ds26gte commented 8 years ago

Fixed in ds26gte/code.pyret.org@5fd20b3.

Ensure top-level defines of non-lambda expressions remain in stable order.

schanzer commented 8 years ago

Will close once I can confirm - looks like the current instance doesn't reflect these changes.

ds26gte commented 8 years ago

Sorry: Please try now. (Fix in ds26gte/code.pyret.org@c80a9f9.)

schanzer commented 8 years ago

Still not there:

(define x 0) (define (f) x)

I feel like we're playing whack-a-mole. Do you have a set of tests you use? If not, this comment thread is probably a good place to start collecting them. :)

ds26gte commented 8 years ago

Do we disallow all forward references except to define-struct?

schanzer commented 8 years ago

We should emulate WeScheme's behavior, so there are other kinds of forward references are permitted:

I think Eli Barzilay had a nice explanation of this in a stack overflow thread a while back

ds26gte commented 8 years ago

The only troublesome one is:

(define (f x) (g x)) (define (g x) (f x)) is allowed

Pyret won't allow functions to mutually refer to each other (the one above can refer to the one below, but not vice versa). Thus we can't write the canonical even?/odd? pair in terms of each other. Is this too much of a loss?

schanzer commented 8 years ago

Let's proceed without it - I'll open a separate issue to track that, in case we wind up needing it later. Could we not eventually use var and assignment, to basically simulate letrec*-style behavior?

ds26gte commented 8 years ago

OK. ds26gte/code.pyret.org@a27a71f.

It satisfies all the "allows" in your list except for the mutual recursion.

schanzer commented 8 years ago

Confirmed closed! 👍