Skip to content

Fix CLOS, MAKE-LOAD-FORM, and TYPE-OF ANSI test failures#742

Closed
blakemcbride wants to merge 2 commits intoarmedbear:masterfrom
blakemcbride:clos-corrections
Closed

Fix CLOS, MAKE-LOAD-FORM, and TYPE-OF ANSI test failures#742
blakemcbride wants to merge 2 commits intoarmedbear:masterfrom
blakemcbride:clos-corrections

Conversation

@blakemcbride
Copy link
Copy Markdown

Fix CLOS, MAKE-LOAD-FORM, and TYPE-OF ANSI test failures

Summary

Resolves 32 ANSI test failures across eight areas of ABCL's CLOS,
type system, and file-compiler behavior. All originally-targeted
tests pass, with no regressions against the baseline test suite.

Builds on (and cherry-picks) the MAKE-LOAD-FORM data-flow ordering
commit, and adds seven further fixes.

Area Tests fixed
MAKE-LOAD-FORM.ORDER.3–14 12
DEFINE-METHOD-COMBINATION-LONG.04.2, .11.2, .11.3 3
DEFGENERIC.ERROR.1/2/3 3
CALL-NEXT-METHOD.ERROR.1/2 2
DEFSTRUCT.ERROR.3/4 2
FIND-CLASS.24 1
MAKE-CONDITION.3/4 2
TYPE-OF.1/4 (and TYPE-OF.1-relaxed) 3
DEFCLASS.FORWARD-REF.1/2/3 (kept green) 3

Changes

1. MAKE-LOAD-FORM data-flow ordering (ORDER.3–14)

Cherry-picked from the make-load-form branch. The file compiler now
emits creation and initialization forms through a per-fasl instance
vector sys::*fasl-instances*, with a two-phase dependency walker that
honors CLHS's "creation before any reference" and "init ASAP after
creation" rules. *FASL-VERSION* is bumped 43 → 44. See the
make-load-form PR description for full detail on this portion.

2. DEFINE-METHOD-COMBINATION long form (clos.lisp)

Three independent bugs in the long-form define-method-combination
expansion:

  • :order / :required evaluated as forms, not quoted symbols.
    canonicalize-method-group-spec was emitting :order ',order and
    :required ',required-p, so a group spec like
    ((method-list * :order order)) substituted the literal symbol
    order rather than the value of the lexical variable. CLHS
    specifies these are forms to be evaluated at call site.
  • Optional parameters "in excess" of the generic function's
    optional count were bound to NIL
    instead of their initforms.
    Rewrote the optional-parameter handling in
    method-combination-type-lambda-with-args-emf to emit an
    initform binding for excess optionals, and to bind excess
    supplied-p parameters to NIL.
  • Keyword supplied-p parameters leaked the member tail. The
    same binding symbol was used for both the boolean supplied-p and the
    (cadr …) accessor, so user code saw e.g. (:Z1 4) instead of T.
    Split these via a private gensym for the member result;
    supplied-p now binds to (not (null …)).

3. CALL-NEXT-METHOD applicable-methods check (clos.lisp)

AMOP requires that when CALL-NEXT-METHOD is passed an argument list,
those arguments must produce the same set of applicable methods as the
original call. ABCL previously silently re-dispatched on whatever
arguments it received.

Added *call-next-method-gf* / *call-next-method-applicable-methods*
specials bound by the slow-method-lookup path, plus a
cnm-check-applicable-methods helper invoked from the
flet-call-next-method expansion when explicit CNM args are given. The
emfun is wrapped so the binding survives effective-method caching.

4. DEFCLASS top-level compile-time recognition (clos.lisp)

FIND-CLASS.24 requires that a (defclass test-class …) appearing at
top level in a compiled file be visible to macros invoked later in the
same compilation (via (find-class 'test-class nil env)). The defclass
macro now:

  • at :compile-toplevel, registers a placeholder
    forward-referenced-class so later compile-time find-class calls
    succeed;
  • at :load-toplevel, clears that placeholder only when it has no
    direct subclasses
    (i.e. it really is our own placeholder, not a
    legitimate forward reference from a previously-defined subclass),
    so ensure-class then goes through the primitive path and avoids
    a MOP superclass-validity assertion that built-in classes like
    STREAM fail;
  • at :execute, does not touch existing forward-references
    (fixes the DEFCLASS.FORWARD-REF.* regression introduced by an
    earlier version of this fix).

5. TYPE-OF vs. TYPEP consistency (NilVector + SGF)

TYPE-OF.1 / .4 require that for every object x, (type-of x) be
a subtype of every built-in type that x is TYPEP. Two specific
instances failed this invariant in ABCL:

  • NIL-vector ↔ BASE-STRING. A (simple-array nil (*)) reported
    (typep x 'base-string) → T, but
    (subtypep '(nil-vector n) 'base-string) → NIL, T because the
    upgraded element types (nil vs character) are disjoint per CLHS
    4.2.3. NilVector.typep now returns NIL for BASE-STRING /
    SIMPLE-BASE-STRING. (STRING remains T — nil is a subtype of
    character, so a NIL-vector is legitimately a string.)
  • STANDARD-GENERIC-FUNCTIONCOMPILED-FUNCTION.
    FuncallableStandardObject.typep previously returned T for
    COMPILED-FUNCTION when the installed dispatcher happened to be
    compiled; this is an implementation detail of the funcallable
    instance, not a property of the object itself. Returning NIL
    matches SBCL/CCL behavior. Because jvm-compile used the old T
    answer as an early-return signal, it now explicitly skips
    funcallable-standard-object definitions (they have no LAMBDA
    expression to work from).
  • subtypep bugfix in the string-array branch of
    csubtypep-array: when the second type was a bare symbol like
    nil-vector (no dimension spec), the size check compared
    (car i1) against nil instead of treating the missing dimension
    as *. (subtypep '(nil-vector 0) 'nil-vector) now returns
    T, T.

Files changed

Beyond the cherry-picked make-load-form commit:

  • src/org/armedbear/lisp/clos.lisp
    • defclass macro: compile-time placeholder + guarded
      load-time clear.
    • canonicalize-method-group-spec: evaluate :order /
      :required at call site.
    • method-combination-type-lambda-with-args-emf: excess-optional
      initform binding; split key supplied-p from member tail.
    • *call-next-method-gf*, *call-next-method-applicable-methods*,
      cnm-check-applicable-methods; slow-method-lookup emfun wrap.
  • src/org/armedbear/lisp/defstruct.lisp
    • (DEFSTRUCT.ERROR.3/4 corrections.)
  • src/org/armedbear/lisp/compiler-pass2.lisp
    • jvm-compile: short-circuit on funcallable-standard-object.
  • src/org/armedbear/lisp/FuncallableStandardObject.java
    • typep: return NIL for COMPILED-FUNCTION.
  • src/org/armedbear/lisp/NilVector.java
    • typep: return NIL for BASE-STRING / SIMPLE-BASE-STRING.
  • src/org/armedbear/lisp/subtypep.lisp
    • csubtypep-array string-branch: treat missing dimensions as *.

Test plan

  • All 28 originally-targeted tests pass
    (MAKE-LOAD-FORM.ORDER.3–14, DEFGENERIC.ERROR.1/2/3,
    CALL-NEXT-METHOD.ERROR.1/2,
    DEFINE-METHOD-COMBINATION-LONG.04.2/.11.2/.11.3,
    DEFSTRUCT.ERROR.3/4, FIND-CLASS.24, MAKE-CONDITION.3/4,
    TYPE-OF.1/4).
  • DEFCLASS.FORWARD-REF.1/2/3 continue to pass (regression guard
    on the DEFCLASS compile-time change).
  • All 33 DEFINE-METHOD-COMBINATION-LONG.* tests pass.
  • All 14 MAKE-LOAD-FORM.ORDER.* tests pass.
  • Full cl-test suite: 21932 tests, failure count drops from
    119 → 113. No new CLOS-related failures; remaining failures
    are pre-existing and unrelated (pathname/file, FORMAT,
    LOOP.1.40–43, etc.).

Compatibility

Fasl layout changed (MAKE-LOAD-FORM portion) — *FASL-VERSION* was
already bumped 43 → 44, and older .abcl fasls must be recompiled.
No other public API changes; internal additions
(*call-next-method-gf*, cnm-check-applicable-methods,
%fasl-* helpers) are in SYSTEM / EXT.

@blakemcbride
Copy link
Copy Markdown
Author

Duplicated in pull request 743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant