Skip to content

Re-working <https://github.com/armedbear/abcl/pull/743> from <https://github.com/blakemcbride/abcl/tree/conditions-packages-types-structs>#748

Open
easye wants to merge 11 commits intoarmedbear:masterfrom
easye:conditions-packages-types-structs
Open

Re-working <https://github.com/armedbear/abcl/pull/743> from <https://github.com/blakemcbride/abcl/tree/conditions-packages-types-structs>#748
easye wants to merge 11 commits intoarmedbear:masterfrom
easye:conditions-packages-types-structs

Conversation

@easye
Copy link
Copy Markdown
Collaborator

@easye easye commented Apr 21, 2026

IN-PROGESS

blakemcbride and others added 3 commits April 23, 2026 10:16
TODO: reword commit, at least noting fasl incompatiblity at the top.

- Fixes all 12 failing `MAKE-LOAD-FORM.ORDER.*` tests (ORDER.3–14) in
  the ANSI test suite: compiled-test failures drop from 64 to 52
  with no regressions.
- The file compiler now emits `MAKE-LOAD-FORM` creation and
  initialization forms to the fasl in data-flow dependency order,
  with init forms run ASAP per CLHS.
- Bumps `*FASL-VERSION*` from 43 to 44 (fasl layout changed).

CLHS requires, for every literal instance handled by
`MAKE-LOAD-FORM`:

- the creation form runs after creation of every object it
  references (transitive closure);
- the init form runs after all objects it references are created,
  and as soon as its data-flow dependencies are satisfied.

Previously, ABCL emitted the creation + init forms inline at the
point where the instance was referenced, expanded via
`get-instance-form` as `(let ((inst <creation>)) <init> inst)`. This
could not express cross-object dependencies and failed every
`MAKE-LOAD-FORM.ORDER.*` test that involved more than one instance
with dependencies between them.

Each fasl now carries a per-file instance vector
`sys::*fasl-instances*` indexed by integer. For every literal
instance referenced in the compile unit, the compiler emits (in
dependency order, *before* the top-level form that references it):

    (setf (svref sys::*fasl-instances* N) <creation-form>)
    <initialization-form>

and the original inline reference to the instance is rewritten to
`#.(svref sys::*fasl-instances* N)`. The vector is allocated once in
the fasl prologue from the final instance count.

A two-phase dependency walker drives the emission:

1. `%fasl-ensure-created(X)` — recursively ensures creation of every
   instance embedded in `X`'s creation form, then emits `X`'s
   creation form. A re-entry while `X` is mid-creation is a real
   circular creation dependency and signals an error (CLHS: undefined
   behavior).
2. `%fasl-ensure-initialized(X)` — ensures `X` is created, then
   recursively ensures initialization of every instance embedded in
   `X`'s init form, then emits `X`'s init form. An in-progress init
   cycle is tolerated (CLHS leaves ordering within an init cycle
   unspecified).

To honor the ASAP rule (tests `.6`, `.10`–`.12`),
`%fasl-ensure-created` eagerly initializes `X` immediately after
emitting its creation form *iff* `%fasl-init-deps-ready-p` reports
that every object referenced by `X`'s init form (ignoring `X` itself,
since MAKE-LOAD-FORM methods commonly embed `',x` in their init
form) is already initialized. Otherwise init-`X` is emitted later,
either from the top-level walk that directly references `X` or from
another object's init-form walk.

`dump-instance` now writes `#.(svref sys::*fasl-instances* N)`
instead of the legacy inline form. The existing `*circularity*`
walker (`df-check-object`) had to be taught to walk *the same* cons
that `dump-instance` ultimately writes, so a single reference cons
per instance is cached in `*fasl-instance-refs*` and reused by both
sites.

A new dynamic variable `*fasl-emitting-to-fasl-stream*` gates the new
behavior: when unbound (e.g. dumping constants into a class file via
`SERIALIZE-OBJECT`), `dump-instance` / `df-check-instance` fall back
to the legacy `get-instance-form` expansion.
TODO: test, understand and write a better summary comment

CLHS: the :predicate option is limited to use with structures
that have no :type option or are typed and :named.  A non-default
(user-supplied) predicate name is a symbol or NIL; the default is
a string placeholder.

CLHS: if :type is supplied and :named is supplied, the type must
be able to hold the structure's name (a symbol).
TODO: verify this is what we want

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).

A funcallable-standard-object (e.g. a generic function) is not
itself a COMPILED-FUNCTION regardless of whether its installed
dispatcher function happens to be compiled: COMPILED-FUNCTION
characterizes the object, not an internal implementation detail.
Returning T here breaks TYPE-OF invariants (SUBTYPEP of the
object's type would have to cover COMPILED-FUNCTION).
@easye easye force-pushed the conditions-packages-types-structs branch from 367a7dc to a008e76 Compare April 23, 2026 08:18
Mark Evenson added 7 commits April 23, 2026 10:19
- **`STANDARD-GENERIC-FUNCTION` ↔ `COMPILED-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).
TODO: verify

`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:

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
and  SIMPLE-BASE-STRING.  STRING remains T — NIL is a subtype of
character, so a NIL-vector is legitimately a string.
TODO: verify, reword

- **`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`.
TODO: split off CALL-NEXT-METHOD fixes

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 …))`.

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.

TODO --- SPLIT?

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.
CLHS specifies that multiple :nicknames options are to be merged.
DEFPACKAGE was using `setq`, so only the *last* `:nicknames` clause
survived:

    (defpackage :foo (:nicknames :a) (:nicknames :b))  ; only :B seen
TODO: Find correct CLHS citation as §2.4.8.7
<http://clhs.lisp.se/Body/02_dhg.htm> does not require signalling a
reader error as noted below.

CLHS 2.4.8.7 requires `#:<token>` to signal a reader error if the
token contains an unescaped package marker (a colon not preceded by
`\` or inside a `|…|` escape). ABCL previously accepted `#:foo:bar`
silently and constructed a symbol whose name contained a colon.

`Stream.readSymbol` now scans the token post-`_readToken`, consulting
the per-character escape `BitSet` returned by the tokenizer, and
signals `ReaderError` on any unescaped `:`. The check is skipped
when `*read-suppress*` is true, so `READ-SUPPRESS.SHARP-COLON.7`
continues to pass (it reads `"#::"` with `*read-suppress*` = T and
expects `NIL`, no error).
Addresses problems with the ANSI-TEST DISASSEMBLE.ERROR.3 expecting
TYPE-ERROR for invalid arguments.

Note: an improved version of this sort of checking would make the
effort to see if a given symbol was defined in the function space, if
a given SETF expander had a definition, etc.  SBCL seems to do this.
@easye easye force-pushed the conditions-packages-types-structs branch 5 times, most recently from 63560af to ceb4b36 Compare April 24, 2026 15:18
CLHS defines APROPOS and APROPOS-LIST with the lambda-list

    (string-designator &optional package-designator)

ABCL extends ANSI by adding a third EXTERNAL-ONLY parameter to list
only external symbols.

    (string-designator &optional package-designator external-only)
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.

2 participants