AccelerationNet / access

A common lisp library to unify access to common dictionary-like data-structures
Other
84 stars 12 forks source link

Access structure-objects #20

Closed mmontone closed 6 months ago

mmontone commented 1 year ago

This pull request allows accessing structure objects, that is not possible at the moment.

phoe commented 1 year ago

You might want to add a documentation comment saying that doing this is UB by the language specification, but all most-used contemporary implementations (SBCL, CCL, ECL, Clasp, CLISP, ABCL, LW, ACL) define slot-value on structure-objects in a meaningful and expected way.

mmontone commented 1 year ago

Yes, this is not ready. I should add tests and test on all implementations. Will do that soon.

phoe commented 1 year ago

Your patch should Just Work™ as-is without much trouble, but yes, adding some basic tests that verify this behavior would be useful for cross-implementation support.

mmontone commented 1 year ago

Right

El sáb., 29 oct. 2022 17:35, phoe @.***> escribió:

Your patch should Just Work™ as-is without much trouble, but yes, adding some basic tests that verify this behavior would be useful for cross-implementation support.

— Reply to this email directly, view it on GitHub https://github.com/AccelerationNet/access/pull/20#issuecomment-1295966327, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADKPDU2ZRR6DWDYPLW4GXTWFWDAJANCNFSM6AAAAAARRP5LXU . You are receiving this because you authored the thread.Message ID: @.***>

mmontone commented 1 year ago

Added tests.

They pass for SBCL and ECL, but there's an error in CCL. My ABCL doesn't load lisp-unit2. I'll test again with latest version.

mmontone commented 1 year ago

I've fixed the tests for CCL.

mmontone commented 1 year ago

My ABCL v1.9.0 (latest released) won't load cl-unicode, that is needed by lisp-unit2, that is needed by access-test. So I'm not being able to run tests from ABCL for now..

bobbysmith007 commented 1 year ago

Thanks for all the work on this so far and the cross implementation testing. Is it ready to be merged?

bobbysmith007 commented 1 year ago

Phoe, if you have the ability to test on multiple implementations, and are interested, I could make you a committer / merger on this project. I haven't had much opportunity to program in Common LISP in the past few years.

mmontone commented 1 year ago

Thanks for all the work on this so far and the cross implementation testing. Is it ready to be merged?

Tests pass for SBCL, CCL, ECL. Coundnt' test on ABCL because lisp-unit2 doesn't load for me.

I have some doubts of what I did in line 538, but I don't know what to do about it atm.

mmontone commented 1 year ago

I've updated do-set-access for structure-objects. Just use (setf slot-value) with actual-slot-name. I think that's correct.

Compiles without warning and tests pass for SBCL, CCL, ECL.

If you decide to merge, feel free of squashing commits.

I think I'm done here :)

challmand commented 1 year ago

sad this is being overlooked.

phoe commented 1 year ago

Phoe, if you have the ability to test on multiple implementations, and are interested, I could make you a committer / merger on this project. I haven't had much opportunity to program in Common LISP in the past few years.

@bobbysmith007 I think I can do that, yes. I think I can do basic testing (via Roswell's ability to download implementations) and merge PRs.

bobbysmith007 commented 1 year ago

@phoe , thanks for your offer. I have added you as a maintainer, so you should be able to merge this whenever you are ready. If you run into any problems, or if it needs to be merged more readily than that, I can do so, but it feels strange to merge code I cannot currently run locally (I dont currently have a common lisp environment running)

vindarel commented 6 months ago

Hello, friendly ping. Thanks to all involved :pray:

phoe commented 6 months ago

Thanks for the reminder - merging this now.