ANSI-TEST fixes #261
Open
ANSI-TEST fixes #261
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The failing test's description: ;;; Test that SHIFTF returns a single value, even though the first ;;; place has multiple values. The form: (let ((x 'a) (y 'b)) (values (multiple-value-list (shiftf (values x y) (floor 10 3))) x y)) Was expected to return the following three values: (A) 3 1 But instead evaluated to: (A B) 3 1
The failing test's description: ;;; &whole is followed by a destructuring pattern (see 3.4.4.1.2) The form: (macrolet ((%m (&whole (m a b) c d) `(quote (,m ,a ,b ,c ,d)))) (%m 1 2) Was expected to return the following value: (%M 1 2 1 2) But instead signaled an error, as MACROLET did not expect &WHOLE to be followed by a destructuring pattern.
The failing test's description: ;;; Uninterned symbols must not contain a package prefix (see CLHS 2.4.8.5) The form: (read-from-string "#:a:b") Was expected to signal a READER-ERROR, but signaled a SIMPLE-ERROR instead.
The body of the failing test:
(deftest compile-file.16
(let* ((file #p"compile-file-test-file-5.lsp")
(target-pathname (compile-file-pathname file))
(*compile-print* nil)
(*compile-verbose* nil))
(when (probe-file target-pathname)
(delete-file target-pathname))
(compile-file file)
(load target-pathname)
(values
(equalpt-or-report (truename file) (funcall 'compile-file-test-fun.5))
(equalpt-or-report (pathname (merge-pathnames file))
(funcall 'compile-file-test-fun.5a))))
t t)
Contents of file `compile-file-test-file-5.lsp`:
(in-package "CL-TEST")
(defun compile-file-test-fun.5 ()
'#.*compile-file-truename*)
(defun compile-file-test-fun.5a ()
'#.*compile-file-pathname*)
This rest is supposed to return VALUES T T but instead returns:
T
(#P"/home/phoe/Projects/Git/ansi-test/sandbox/compile-file-test-file-5.lsp"
#P"home:Projects;Git;ansi-test;sandbox;compile-file-test-file-5.lsp.newest")
The spec says:
The value of *COMPILE-FILE-PATHNAME* must always be a pathname or nil.
So *COMPILE-FILE-PATHNAME* is allowed to be a physical OR a logical
pathname, BUT:
During a call to COMPILE-FILE, *COMPILE-FILE-PATHNAME* is bound to the
pathname denoted by the first argument to COMPILE-FILE, merged against the
defaults; that is, it is bound to (PATHNAME (MERGE-PATHNAMES INPUT-FILE)).
The spec says on MERGE-PATHNAMES:
MERGE-PATHNAMES returns a logical pathname if and only if its first arcument
is a logical pathname, or its first argument is a logical pathname
namestring with an explicit host, or its first argument does not specify a
host and the default-pathname is a logical pathname.
First argument is #p"compile-file-test-file-5.lsp" and therefore has no host.
The defaults for me are`#P"/home/phoe/Projects/Git/ansi-test/sandbox/"` - a
physical pathname. So the first call MERGE-PATHNAMES properly returns a
physical pathname.
And therefore, inside the compiled file, the value of *COMPILE-FILE-PATHNAME*
is incorrect. We solve this by modifying CCL::%COMPILE-FILE and removing an
explicit but unnecessary (and non-conforming) call to
CCL::BACK-TRANSLATE-PATHNAME that modifies the value that is then bound to
*COMPILE-FILE-PATHNAME* inside the compiled file.
The body of the failing test:
(deftest subtypep.cons.43
(let* ((n -3.926510009989861d7)
(t1 '(not (cons float t)))
(t2 `(or (not (cons (eql 0) (real ,n ,n)))
(not (cons t (eql 0))))))
(multiple-value-bind
(sub1 good1)
(subtypep* t1 t2)
(multiple-value-bind
(sub2 good2)
(subtypep* `(not ,t2) `(not ,t1))
(or (not good1)
(not good2)
(and sub1 sub2)
(and (not sub1) (not sub2))))))
t)
We are only interested in a part of this test. After simplifying:
(let* ((t1 '(not (cons float t)))
(t2 `(or (not (cons (eql 0) (real -3.5d0 -3.5d0)))
(not (cons t (eql 0))))))
(subtypep t1 t2))
This evaluates to (VALUES NIL T), even though T2 is equivalent to T and
therefore the valid results are (VALUES T T) and (VALUES NIL NIL).
After trying to find a smaller reproducible test case, I have found
that the error is triggered with the following two types:
CCL> (csubtypep (specifier-type
'(CONS (NOT FLOAT) T))
(specifier-type
`(OR (CONS (INTEGER 0 0)
(OR (REAL * (-3.5D0))
(INTEGER * -1)
(NOT INTEGER)
(REAL (-3.5D0))))
(CONS (OR (NOT INTEGER)
(INTEGER * -1)
(INTEGER 1))
T))))
;=> NIL, T
Simplifying again:
CCL> (specifier-type '(OR (REAL * (-3.5D0))
(NOT INTEGER)
(REAL (-3.5D0))))
The above type is equivalent to T, but CCL fails to infer that fact.
We fix this issue by modifying CCL::SIMPLIFY-UNIONS and introducing
a special case for merging numeric types that are neighboring and have
a common exclusive bound (one lower, one higher) in presence of other types
that might contain that exclusive bound. This causes CCL to correctly
simplify the above type into T.
The test in question is:
(deftest map.error.11
(let ((type '(or (vector t 5) (vector t 10))))
(if (subtypep type 'vector)
(eval `(signals-error (map ',type #'identity '(1 2 3 4 5 6))
type-error))
t))
t)
This fails because (coerce '(1 2 3 4 5 6) '(or (vector t 5) (vector t 10)))
does not signal an error in safe code. We "fix" this issue by following
SBCL's lead in the matter and signaling an error whenever an OR type is passed
to COERCE and the expected type is a subtype of VECTOR; a proper fix is not in
place due to the amount of work required to perform the typecheck.
This in turn causes ANSI-TEST MAP.48 to break. It expects the following form
(map '(or (vector t 10) (vector t 5)) #'identity '(1 2 3 4 5))
to return #(1 2 3 4 5). We work around this issue by disabling note
:RESULT-TYPE-ELEMENT-TYPE-BY-SUBTYPE in ANSI-TEST.
Signed-off-by: Michał phoe Herda <[email protected]>
This commit removes CCL's implementation of ~F FORMAT control and the CCL copy of FLONUM-TO-STRING, replacing these with their respective SBCL counter parts. This was made in order to make all ANSI-TESTs from the FORMAT.F.* group to pass. This commit additionally modifies the internals of PRINT-OBJECT method for floats in order to account for the modified FLONUM-TO-STRING.
CCL incorrectly formatted (format nil "~2f ~2f" 1.1 1.9) - the proper result
is "1.0 2.0" due to the reasoning provided in ANSI-TEST:
Rationale for FORMAT.F.45: CLHS 22.3.3.1 states that "d is the number of
digits to print after the decimal point;" AND that the number is printed
"rounded to d fractional digits".
If we want to print 1.1 in a field of width 0, then we compute D to be W,
minus the number of digits to be printed before the decimal point, minus 1
for the decimal point. So, in this case, D = W - 1 - 1 = 0.
This means that we must print exactly 0 digits after the decimal point and
that we must round the number to 0 fractional digits. The latter is doable
and we round 1.1 to 1.0, BUT the first contradicts the first paragraph of
22.3.3.1 which states, "arg is printed as a float". "1." is not a float in
Common Lisp, since it is read as the integer 1 in base 10.
Therefore, in order to work around this corner case, we explicitly print one
decimal digit so that the resulting number is recognizable as a float. Since
the number was rounded to 0 decimal digits, then this digit is 0. This way,
we get "1." concatenated with "0" that gives us "1.0".
CCL incorrectly printed 0.01 with ~1f and ~0f as ".01", where the correct value is ".0". This commit fixes this bug.
|
Is this still ready to go? Any reason I shouldn't merge it? |
|
I guess that it is still good to go - can't see any reason why not. |
|
One more thing though, just to be sure - I'd like Clozure/ccl to have Travis configured on it in order to run the ccl-test automated test suite on it, just to be sure we do not break anything. I will prepare a PR with this, however, it'll require a few steps for the repository administrator (read: not me) to authenticate with Travis. |
|
Phoe I get both test-errors solved and new test-errors. Did you change some conditions for the tests, e.g. with notes? I defined rt::expected-failures to get this outpupt. 17 unexpected failures: MAP.48, PRINT.SHORT-FLOAT.RANDOM, PRINT.SINGLE-FLOAT.RANDOM,
PRINT.DOUBLE-FLOAT.RANDOM, PRINT.LONG-FLOAT.RANDOM, FORMAT.F.46B,
FORMATTER.F.46B, FORMAT.E.10, FORMAT.E.11, FORMAT.E.12, FORMAT.E.13,
FORMAT.E.14, FORMAT.E.15, FORMAT.E.22, FORMAT.E.23, FORMAT.E.24, FORMAT.E.25.
14 unexpected successes:
SYNTAX.SHARP-COLON.ERROR.1, SHIFTF.7, MACROLET.36, MAP.ERROR.11,
SUBTYPEP.CONS.43, FORMAT.F.5, FORMAT.F.8, FORMAT.F.45, FORMATTER.F.45,
FORMAT.F.46, FORMATTER.F.46, FORMAT.F.47, FORMATTER.F.47, COMPILE-FILE.16.running the self tests of ccl also produces some messages: ================ Test suite failed ================
8 out of 21908 total tests failed:
CL-TEST::MAP.48, CL-TEST::PRINT.SHORT-FLOAT.RANDOM,
CL-TEST::PRINT.SINGLE-FLOAT.RANDOM, CL-TEST::PRINT.DOUBLE-FLOAT.RANDOM,
CL-TEST::PRINT.LONG-FLOAT.RANDOM, CL-TEST::FORMAT.F.5, CL-TEST::FORMAT.F.8,
CL-TEST::CCL.BUG\#1186. |
|
@kpoeck I remember that I have made some issues on the repository at https://gitlab.common-lisp.net/ansi-test/ansi-test that comment on some of the tests. I am also disabling two of the (disable-note :ansi-spec-problem)
(disable-note :result-type-element-type-by-subtype) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a series of commits that fixes multiple ANSI-TEST failures from the upstream ANSI-TEST.
Please review and test as appropriate. I have tested individual commits on Travis and also built CCL and ran ANSI-TEST on my local machine after cherry-picking the commits for this branch.
No bootstrapping issues are present in this MR.