AlexKnauth / afl

a racket lang-extension for rackjure-like anonymous function literals
MIT License
6 stars 2 forks source link

Build fails on pkg-build.racket-lang.org #1

Closed SuzanneSoy closed 7 years ago

SuzanneSoy commented 8 years ago

Hello,

I recently wrote a small package which uses #lang afl, unfortunately it does not build on pkg-build.racket-lang.org, because afl fails to build there:

http://pkg-build.racket-lang.org/server/built/fail/afl.txt

…
raco setup: making: <pkgs>/afl/afl/tests
raco setup:  in <pkgs>/afl/afl/tests
/home/racket/build-pkgs/user/.racket/6.6/pkgs/afl/afl/tests/test-afl-racket.rkt:1:0: bad language path following at-exp
  compilation context...:
   /home/racket/build-pkgs/user/.racket/6.6/pkgs/afl/afl/tests/test-afl-racket.rkt
  context...:
   /home/racket/build-pkgs/racket/collects/syntax/readerr.rkt:15:2: -raise-read-error
   /home/racket/build-pkgs/racket/collects/syntax/module-reader.rkt:295:4: -read-syntax
   /home/racket/build-pkgs/user/.racket/6.6/pkgs/hygienic-reader-extension/hygienic-reader-extension/extend-reader.rkt:11:2
   /home/racket/build-pkgs/user/.racket/6.6/pkgs/afl/afl/lang/reader.rkt:12:5
   /home/racket/build-pkgs/racket/collects/syntax/modcode.rkt:62:2: reader
   /home/racket/build-pkgs/racket/collects/compiler/cm.rkt:634:2: do-check
   /home/racket/build-pkgs/racket/collects/compiler/cm.rkt:722:4
   /home/racket/build-pkgs/racket/collects/compiler/compiler.rkt:169:4: worker
   /home/racket/build-pkgs/racket/collects/compiler/compiler.rkt:92:2: compile-directory-visitor23
   /home/racket/build-pkgs/racket/collects/setup/setup-core.rkt:1038:2: compile-cc
   /home/racket/build-pkgs/racket/collects/setup/setup-core.rkt:1114:10: for-loop
   /home/racket/build-pkgs/racket/collects/setup/setup-core.rkt:70:0: setup-core
   /home/racket/build-pkgs/racket/collects/setup/setup.rkt:65:3
   /home/racket/build-pkgs/racket/collects/pkg/main.rkt:17:0: setup
   (submod /home/racket/build-pkgs/racket/collects/pkg/main.rkt main): [running body]
AlexKnauth commented 8 years ago

I noticed that on pkg-build too, but when I run it on my computer it passes

SuzanneSoy commented 8 years ago

What troubles me is that at-exp is only used afl/tests/test-afl-at-exp-racket.rkt, but the error is raised when compiling afl/tests/test-afl-racket.rkt.

I made a fork in which I removed afl/tests/test-afl-at-exp-racket.rkt, to see if afl/tests/test-afl-racket.rkt still fails on its own with tomorrow's build:

https://pkgd.racket-lang.org/pkgn/package/afdl

If it succeeds, could it be some weird form of caching which is happening on drdr?

SuzanneSoy commented 8 years ago

I managed to reproduce the bug on my machine:

rm ~/.racket/snapshot/pkgs/afl/afl/tests/compiled -fr
raco setup -j 1 --pkgs afl

I tried removing the afl/tests/test-afl-at-exp-racket.rkt file, and afl/tests/test-afl-racket.rkt compiles fine, but then test-afl-scribble.rkt gives an error:

/home/georges/.racket/snapshot/pkgs/afl/afl/tests/test-afl-scribble.rkt:25:22: read: unexpected `}'

It seems to me that when two files using #lang afl are compiled in sequence by raco setup, the first one's reader is "cached" in some way. When compiling several times in a row, the files with previously-generated bytecode are skipped, and therefore the file which failed then compiles successfully.

AlexKnauth commented 8 years ago

Ok, is there any way you know of to disable that caching behavior and make sure that it works whether they're compiled separately or together?

(Separate-compilation-guarantee, except for readers)

SuzanneSoy commented 8 years ago

I have absolutely no idea… I think it's best to ask on the mailing list, or to directly file a bug against raco (it really does not sound like intended behaviour).

I have had some issues with raco test in the past, where some tests do not run in complete isolation, too.

SuzanneSoy commented 7 years ago

I think I found "a" cause of the problem: the lang-extension->lang-reader function in hygienic-reader-extension stores the old read, read-syntax and get-info using (unless x (set! x v)). Changing this so that it always performs (set! x v) allowed me to build afl successfully.

I added some debugging prints, and this is the trace of what happens:

compiling file1.rkt
(set! ?read-syntax #<procedure:-read-syntax>) ;; ?read-syntax was #f
(set! ?read-syntax #<procedure:-read-syntax>) ;; ?read-syntax was #f

compiling file2.rkt
(set! ?read-syntax #<procedure:lang:read-syntax>) ;; ?read-syntax was #<procedure:-read-syntax>
(set! ?read-syntax #<procedure:lang:read-syntax>) ;; ?read-syntax was #<procedure:-read-syntax>

compiling file3.rkt
(set! ?read-syntax #<procedure:lang:read-syntax>) ;; ?read-syntax was #<procedure:lang:read-syntax>
(set! ?read-syntax #<procedure:lang:read-syntax>) ;; ?read-syntax was #<procedure:lang:read-syntax>

I think this shows the "caching" behaviour in action (i.e. the result of lang-extension->lang-reader is cached, but the (λ (old-read) …) is called with different values for old-read.

Since during compilation only the read-syntax part is used, it means that the other functions would throw "?read hasn't been initialized" and "?get-info hasn't been initialized" if they were used by the lang-extension function in some way.

It therefore seems safe to me to just fill in the relevant function (one of read, read-syntax or get-info) when calling the lang-extension function, and make the other two raise an error (as an error will already be raised with the current behaviour). In other words, instead of using delayed functions, we would call the lang-extension function immediately, using unknown-fn to make a function which throws when used in the other two cases:

(λ (old-read)
     (define/lang-reader [-read -read-syntax -get-info]
       (lang-extension (make-lang-reader old-read
                                         (unknown-fn 'read 'read-syntax)
                                         (unknown-fn 'read 'get-info))))
     -read)

The (λ (old-read-syntax) …) and (λ (old-get-info) …) would be defined in the same way.

I submitted a Pull Request at https://github.com/AlexKnauth/hygienic-reader-extension/pull/2 which applies these changes. Please review it with care before merging, though, as I might have broken some invariant or not accounted for some behaviour.

AlexKnauth commented 7 years ago

What if I want the read-syntax function to be able to depend in some way on the old get-info function? Is there a way to do that? (Edit: sorry, meant to comment on the other repository)