Skip to content

Fixes 2024 12 take 1#5

Draft
enometh wants to merge 28 commits into
bohonghuang:masterfrom
enometh:fixes-2024-12-take-1
Draft

Fixes 2024 12 take 1#5
enometh wants to merge 28 commits into
bohonghuang:masterfrom
enometh:fixes-2024-12-take-1

Conversation

@enometh

@enometh enometh commented Dec 25, 2024

Copy link
Copy Markdown

Hello, here are some more miscellaneous patches for your review., do let
me know if these are correct or should be done differently.

bohonghuang and others added 27 commits October 2, 2023 09:54
@enometh enometh force-pushed the fixes-2024-12-take-1 branch from d4d5ef5 to 99bd221 Compare December 26, 2024 04:23
@bohonghuang

bohonghuang commented Dec 26, 2024

Copy link
Copy Markdown
Owner

Hello Madhu! Could you please add test cases for the modifications you've made to test/package.lisp? Since the current cffi-object works well with claw-raylib, I might not be fully aware of the contexts in which these changes are relevant. It would be great to have some intuitive test cases that demonstrate what didn't work in previous versions but does work in your committed version.

…-existent slots

* macros.lisp: (define-package-cobject-classes): Unwrap the loop which
pushes forward declarations for slots and correctly handle slots which
are foreign pointer typedefs: Do not try to find slots when "class"
doesn't have slots. i.e. when "class" is a cffi::foreign-pointer-type,
a typedef of a pointer to a struct. Push the fwd decls of "pointed-to"
class instead.
@enometh enometh force-pushed the fixes-2024-12-take-1 branch from 99bd221 to 5fb2b77 Compare December 28, 2024 14:49
@enometh

enometh commented Dec 28, 2024

Copy link
Copy Markdown
Author

Sorry the earlier commits were bogus. They did not fix the actual problem I was facing with cobj::define-package-cobject-classes. I think I've addressed the problem just now. I'm still working on the tests to go with it.. To demonsrate the problem I'll need a patch which lets us specify the order in which definitions are processed by cobj::define-package-cobject-classes.
0001-sorted-symbol-list-kludge.patch.txt
test:

(defpackage "FOO-TMP" (:use "CL"))
(in-package "FOO-TMP")
(cffi:defcstruct (point :size 8))
(cffi:defctype point-ptr
    (:pointer (:struct point)))
(cffi:defcstruct point-overlay
  (under point-ptr))
(cffi:defcstruct point
  (x :int)
  (y :int))
;;; invoke it with
(unwind-protect
     (progn (setq cobj::*cobject-class-definitions* nil)
	    (setq cobj:*sorted-symbol-list* '(point-overlay point point-ptr))
	    (cobj::define-package-cobject-classes "FOO-TMP"))
  (setq cobj:*sorted-symbol-list* nil))

Before this patch is applied on executing the above form, cffi-object bombs out on sbcl
with

Execution of a form compiled with errors.
Form:
  (CFFI-OBJECT::DEFINE-STRUCT-COBJECT-CLASS
 (POINT-OVERLAY #<POINT-OVERLAY-TCLASS POINT-OVERLAY>))
Compile-time error:
  during macroexpansion of
(CFFI-OBJECT::DEFINE-STRUCT-COBJECT-CLASS (POINT-OVERLAY #)). Use
*BREAK-ON-SIGNALS* to intercept.

 Cannot find the CFFI object class for type POINT.

Any comments appreciated, thankx

@enometh enometh force-pushed the fixes-2024-12-take-1 branch from 5fb2b77 to 1d96ab8 Compare December 28, 2024 15:11
@enometh

enometh commented Dec 28, 2024

Copy link
Copy Markdown
Author

I pushed the wrong commit, hopefully I've pushed the correct branch now.. Can you please delete the earlier files pushed on the branch.

@enometh enometh force-pushed the fixes-2024-12-take-1 branch from 1d96ab8 to 21af316 Compare December 28, 2024 15:15
@bohonghuang

bohonghuang commented Dec 28, 2024

Copy link
Copy Markdown
Owner

It seems you are tackling forward references of structs, which was already supported in the previous version of cffi-object with define-prototype-cobject-class, but you need to define with (define-cobject-class <package-name>). The macro will sort all the definitions based on the dependencies among the structural types within that package.

@enometh

enometh commented Dec 30, 2024

Copy link
Copy Markdown
Author

The proposed patch only fixes a type mismatch in the implementation.

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