clasp-developers / clasp

clasp Common Lisp environment
https://clasp-developers.github.io/
2.58k stars 145 forks source link

interface for cando #1155

Open Bike opened 3 years ago

Bike commented 3 years ago

Since cando and clasp have grown out of the same project, cando uses clasp "internals" quite generously. It would be more convenient for maintenance if cando used a defined interface to clasp, so that what cando uses can be specially protected during clasp development.

From a quick grep, the cando lisp code has 192 uses of the core: package. (The cando C++ code's use of clasp internals will be a whole other kettle of fish.) Those uses are as follows:

There are also a few uses of core:*read-hook* and related, but hopefully those are being removed soon.

kpoeck commented 3 years ago

As a tiny start, we already export from ext the following (in init.lisp )

since I found them in either:

yitzchak commented 3 years ago

If the use of core:make-cxx-object is to create an object in the chem package I suspect that it is just missing some kind of make-? function or is a reference before one was added. Even if we still use the equivalent of core:make-cxx-object we probably want to at least hide it behind a lisp make-? function for that sake of end users.

yitzchak commented 3 years ago

For instance (chem:make-aggregate) is available versus (core:make-cxx-object 'chem:aggregate)

drmeister commented 3 years ago

This is really nice - please proceed to simplify the interface but don't break anything.

yitzchak commented 3 years ago

FYI the leap-command-line-* functions are actually in core. https://github.com/cando-developers/cando/blob/2614173783b025bec7f9596f7467258473a15936/src/main/extension.cc#L80

Maybe they should be moved?

myonkunas commented 3 years ago

The use of core:*read-hook* has been tested with minimal performance degradation and removed. Merged here: https://github.com/cando-developers/cando/commit/883a6298c98c6cea45c15e302ee43742cbacac91

Bike commented 3 years ago

In 7e0724867e79bd428fe4bbcc2dc34079b329b34d I have created a clasp-posix package. For the moment it just reexports some stuff from core (and ext) and those symbols remain where they are, so it's backward compatible

yitzchak commented 3 years ago

I can start rolling in the clasp-posix package into my open PR over in cando.

Bike commented 3 years ago

Whoops, I didn't export the sigset stuff cando uses. Done in 5cdabf66e1de344b68ab8f879da37c9ab64378ed. I also didn't export num-logical-processors since it's not actually part of posix (or at least not under that name)

Some of these names could probably be shortened. "sigset-sigaddset"? but that can be for later.

yitzchak commented 3 years ago

What about getpid?

yitzchak commented 3 years ago

I guess I could get it from ext?

Bike commented 3 years ago

i have exported that from clasp-posix as well now

yitzchak commented 3 years ago

Instead of exporting core:make-cxx-object in another package what about just defining a make-instance method?

(defmethod make-instance ((class core:cxx-object) &rest initargs &key &allow-other-keys)
  (apply #'core:make-cxx-object class initargs))

The CLHS says an error should be thrown when make-instance is called on built-in-class because those have restricted capabilities. Instances of cxx-object can be created using make-cxx-object so it seems like having make-instance defined in this case would make sense even though they are a subclass of built-in-class.

I am sure there could be something I don't get about the internal implementation details, but just a thought.

Bike commented 3 years ago

we don't even need to have cxx objects be a subclass of built-in-class, really. we could probably support things like slot-value on them if we wanted to. I don't think there's any standard reason to prevent us from extending make-instance in the fashion you describe.

Unfortunately, while I understand the internal implementations of our CLOS fine, and on that level there is no technical barrier, I haven't really used the CXX stuff much at all and don't have a good idea of how it works.

yitzchak commented 3 years ago

Unfortunately my method doesn't appear to work currently. The method associated with built-in-class seems to take precedence. If cxx-object isn't a subclass of built-in-class then that would probably not be an issue.

Bike commented 3 years ago

Oh, if you're using that method literally, it won't work. make-instance's first argument is the class you're instantiating, not an object. I don't remember what (class-of (find-class 'core:cxx-object)) is off the top of my head, but you want to specialize on that.

Putting methods on make-instance will also implicate the very special optimizations I put on make-instance. They should be suppressed if there's a method, but I haven't tested this extensively.

yitzchak commented 3 years ago

Well that makes sense, (class-of (find-class 'core:cxx-object)) reports #<The STANDARD-CLASS BUILT-IN-CLASS>

Bike commented 3 years ago

I see. In that case we'd have to replace that with a core:cxx-class or something. I think that could be doable. Though it would probably implicate some scraper stuff that's dicey to mess with.