bug#49291: [akater] [PATCH] lisp/emacs-lisp/eieio.el (initialize-instance): Fix initform

[Stefan Monnier]

akater [2021-07-02 07:41:26] wrote:
> A version with correct code generation.  Tests pass on 27.

Sorry 'bout the delay.

> Replace :initform (symbol-value 'x) to :initform x everywhere.

This can't be right because it presumes the CLOS semantics which we
don't have yet: `:initform x` will use the symbol `x` rather than the
value of variable `x` as the default value (and it will emit a warning
because we don't want code to rely on this non-CLOS-compatible
semantics).

Hopefully we'll be able to do that in a few years, but we're not there yet.

> +(defclass eieio-tests-initargs-initform-interplay ()
> +  ((slot-with-initarg-and-initform
> +    :initarg :slot-with-initarg-and-initform
> +    :initform 'value-specified-in-defclass-form)
> +   (slot-with-initarg-only
> +    :initarg :slot-with-initarg-only)
> +   (slot-with-initform-only
> +    :initform 'value-specified-in-defclass-form)))

I don't understand how you can use this to test the interplay
between :initargs and :initform.

I thought this bug was about the fact that :iniform gets evaluated in
cases where it shouldn't, not about the final value in the object.
IOW it's about potential side-effects of evaluating :initform in cases
where we shouldn't.  But the above :initforms are all pure so I can't
see how it can test what we're after.

More to the point: the `eieio-test-25-slot-tests` extended as in your
patch passes successfully without your changes to eieio.el, so it's not
a proper regression test: we want a test that fails on the current code
and succeeds after we install your patch.

Another thing, in the patch you have:

    +(eval-when-compile (require 'cl-macs))

which is not only redundant with the (eval-when-compile (require 'cl-lib))
on the previous line, but makes assumptions about the way cl-lib is
split into files: always require `cl-lib` (rather than `cl-macs`,
`cl-extra`, etc...) when you need any part of it.


        Stefan