Ramarren / cl-parser-combinators

An experimental implementation of parser combinators in Common Lisp
80 stars 5 forks source link

regression on ECL in quicklisp 2013-10-03 #11

Closed avodonosov closed 11 years ago

avodonosov commented 11 years ago

As you can see on the cl-test-grid library page for cl-parser-combinations, http://common-lisp.net/project/cl-test-grid/library/cl-parser-combinators.html, since quicklisp 2013-10-03 the library doesn't build on ECL lisp-to-c compiler.

If you click the failure link, you can find the build output. The error message:

;;; Error:
;;;   in file basic.lisp, position 4783
;;;   at (DEFUN NOTE-POSITION ...)
;;;   * Syntax error in declaration ((INTEGER 0) POSN)

I don't know if the declaration is valid according to CLHS or not. In other words, I don't know if it's a ECL bug or a bug in cl-parser-combinators.

But the fact is, the library doesn't build on ECL now.

avodonosov commented 11 years ago

Opened a ticket for ECL: https://sourceforge.net/p/ecls/bugs/271/

avodonosov commented 11 years ago

Even if this is an ECL bug, it would be good if cl-parser-combinators had a workaround for it, so that the library is working on current ECL versions. The following change allows the library to build on ECL:

diff --git a/basic.lisp b/basic.lisp
index 3db97cd..7eace7b 100644
--- a/basic.lisp
+++ b/basic.lisp
@@ -116,7 +116,7 @@ realising the non-realised ones in the backing store."
 (defparameter *seen-positions-table* nil)

 (defun note-position (posn)
-  (declare ((integer 0) posn)
+  (declare (type (integer 0) posn)
            (special *seen-positions-table*))
   (incf (gethash posn *seen-positions-table* 0)))
Ramarren commented 11 years ago

CLHS says "(typespec var) is an abbreviation for (type typespec var)" so these should be equivalent, which means this appears to be a bug in ECL. I have commited the change to long form.

Additionally my test suite segfaults on ECL, but that appears to be a hu.dwim.stefil issue, as each individual test works, but composite tests segfaults immediately (my check was (parser-combinators-tests::test-parse-string*) and (parser-combinators-tests::test-parse-string*-complete)).

avodonosov commented 11 years ago

Ok.

I am curious about whether the (integer 0) was correct. ECL developers left a comment in the ticket I created. They say that (integer 0) is wrong, because the car of a declaration must be a symbol, according to the http://www.lispworks.com/documentation/HyperSpec/Body/26_glo_d.htm#declaration_identifier

Could you point me to the CLHS chapter where it says that "(typespec var) is an abbreviation for (type typespec var)" ?

Ramarren commented 11 years ago

It says so in http://www.lispworks.com/documentation/HyperSpec/Body/d_type.htm . Also note CLHS issue http://www.lispworks.com/documentation/HyperSpec/Issues/iss349_w.htm , which explicitly says that the intent was to allow complex specifiers.