Subject: Re: [bug#43525] [PATCH] gnu: Add antlr4.

From: Ricardo Wurmus <[email-masked]>
Date: Sun, 20 Sep 2020 08:59:18 +0200

Hi Julien,

> +         (add-before 'check 'remove-graphemes
> +           (lambda _
> +             ;; When running antlr on grahemes.g4, we get a runtime exception:
> +             ;; set is empty. So delete the file that depends on it.
> +             (delete-file "runtime-testsuite/test/org/antlr/v4/test/runtime/java/api/perf/TimeLexerSpeed.java")))

Please let the phase end on #t.  Please also use double spacing in
comments.

> +         (add-before 'build 'generate-unicode
> +           (lambda _
> +             ;; First: build the generator
> +             (invoke "javac" "-cp" (getenv "CLASSPATH")
> +                     "tool/src/org/antlr/v4/unicode/UnicodeRenderer.java"
> +                     "tool/src/org/antlr/v4/unicode/UnicodeDataTemplateController.java")
> +             ;; Then use it
> +             (invoke "java" "-cp" (string-append (getenv "CLASSPATH")
> +                                                 ":tool/src:runtime/Java")
> +                     "org.antlr.v4.unicode.UnicodeRenderer"
> +                     "tool/resources/org/antlr/v4/tool/templates"
> +                     "unicodedata"
> +                     "tool/src/org/antlr/v4/unicode/UnicodeData.java")
> +             (begin
> +               ;; It seems there is a bug with our ST4
> +               (substitute* "tool/src/org/antlr/v4/unicode/UnicodeData.java"
> +                 (("\\\\>") ">"))
> +               ;; Remove the additional file
> +               (delete-file "tool/src/org/antlr/v4/unicode/UnicodeRenderer.java")
> +               #t)))

Why have a BEGIN here?

> +         (add-before 'build 'generate-grammar
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (with-directory-excursion "tool/src/org/antlr/v4/parse"
> +               (for-each (lambda (file)
> +                           (format #t "~a\n" file)

I’m not sure this actually outputs anything.  Does it?  I’m not a fan of
using FORMAT when DISPLAY would work just fine.

> +                           (system* "antlr3" file))

Why not INVOKE?

> +                         '("ANTLRLexer.g" "ANTLRParser.g" "BlockSetTransformer.g"
> +                           "GrammarTreeVisitor.g" "ATNBuilder.g"
> +                           "ActionSplitter.g" "LeftRecursiveRuleWalker.g")))
> +             (with-directory-excursion "tool/src/org/antlr/v4/codegen"
> +               (copy-file "../parse/ANTLRParser.tokens" "ANTLRParser.tokens")

You can use INSTALL-FILE to avoid having to restate the target file name.

> +               (format #t "SourceGenTriggers.g\n")

How about DISPLAY here?

-- 
Ricardo