Possible shadowing going on between types elsa-form and elsa-form-list

[vermiculus]

I'm going through and fixing compile errors that I've found, but I've come across a few issues that I believe needs special attention:

Compiling file /Users/sean/github/elsa/elsa-analyser.el at Wed Jan  2 22:35:09 2019

In elsa--analyse-variable-from-binding:
elsa-analyser.el:86:10:Warning: ‘elsa-form-list-p’ is an obsolete function (as
    of 25.1); use (cl-typep ... '(list-of elsa-form)) instead

In elsa--analyse:lambda:
elsa-analyser.el:459:12:Warning: ‘elsa-form-list-p’ is an obsolete function
    (as of 25.1); use (cl-typep ... '(list-of elsa-form)) instead

In elsa--analyse:quote:
elsa-analyser.el:476:8:Warning: ‘elsa-form-list-p’ is an obsolete function (as
    of 25.1); use (cl-typep ... '(list-of elsa-form)) instead

In elsa--analyse:interactive:
elsa-analyser.el:513:8:Warning: ‘elsa-form-list-p’ is an obsolete function (as
    of 25.1); use (cl-typep ... '(list-of elsa-form)) instead

In elsa--analyse-form:
elsa-analyser.el:655:6:Warning: ‘elsa-form-list-p’ is an obsolete function (as
    of 25.1); use (cl-typep ... '(list-of elsa-form)) instead

I believe this symbol, elsa-form-list-p is being defined by both (defclass elsa-form ...) and (defclass elsa-form-list ...) in elsa-reader.el.

I believe either a new naming convention will need to be established or we'll have to pay special attention with how we order the definitions.

[Fuco1]

I think the error comes from the new EIEIO naming standard. I'm still using the obsolete predicates because they are way more convenient than the cl-type stuff (which I honestly don't understand why it's being preferred over).

[vermiculus]

The obsolete part isn't the simple 'is this thing an object' predicate, it's the 'test if this is a list of these objects' predicate.

At least that was my impression last night.

[Fuco1]

Hmm, but that would be quite an odd function to generate by default. Although it makes sense in the light of the suggested fix

(cl-typep ... '(list-of elsa-form))

I have no idea, but I think this is false positive.

I will probably either define my own predicates or switch to the cl-typep convention anyway since I don't want to run on obsoleted EIEIO for much longer (ideally before 27 is out)

[vermiculus]

I can make those changes pretty easily for you. Ran into one snafoo while doing so, however -- I don't have a good equivalent for (CLASS-NAME-p OBJ NAME) -- what is NAME here? cl-typep has no such argument.

[Fuco1]

I think the name argument is obsolete and I never used it so no problem. It used to be a tag, basically any string at all that was attached to the object for debugging purposes.

I wouldn't move ahead with this just yet though, I have some uncommited code I would need to clean up first.

Also in general I would like to avoid the predicates as much as possible and rework the code so that it actually uses some generic dispatch to handle the cases where I'm checking for type: that's bad OOP.

[vermiculus]

I can definitely wait to see what you come up with instead of predicates -- I'm not too familiar with it, but my dabbling in the Forge project is helping. For CI checking the compile like I mentioned in the PR, I'll just make sure it never fails the build. It'll be easy to turn back on once there's a path forward.

You are using the NAME argument, though -- in quite a few places. This is the only one I can remember off the top of my head:

https://github.com/emacs-elsa/Elsa/blob/f8de89d671ba34f482475b1af51682f983f34f3d/elsa-reader.el#L464

[Fuco1]

Oh... that's confusing :D But that is a custom method I wrote to check if a form is a function call. There is no elsa-form-function-call EIEIO class.

[vermiculus]

Ah, the limits of regex search/replace :-)