search

janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.43k stars 221 forks source link

Use typedefs for strings, symbols, keywords, tuples, structs, and abstracts. #1246

Closed wooosh closed 1 year ago

wooosh commented 1 year ago

This PR replaces generic const uint8_t * and Janet * types with the more specific JanetString/JanetSymbol/JanetKeyword/JanetTuple/JanetStruct/JanetAbstract.

This makes it easier for those who are new to the C API (such as myself) to figure out what these functions actually return/take as arguments. I specifically had issues understanding how the janet_getstring function worked initially because it appeared to return a pointer to the start of the string without a length.

AFAICT the only reason these functions do not use the more specific types is because their function signatures predate the typedefs existing.

I'll send a PR to update the corresponding docs at https://janet-lang.org if this gets merged.

primo-ppcg commented 1 year ago

I can't say if there's a reason why this wasn't done, although it seems reasonable to me.

I think you may have overlooked a few void * to JanetAbstract substitutions:

https://github.com/janet-lang/janet/pull/1246/commits/701913fb190b63e6d8cc52a28bccdc2c86baa390#diff-6b11ee3d77a81a7688781e2c4013c332149289852e0369324b6797a5719ae364L500 https://github.com/janet-lang/janet/pull/1246/commits/701913fb190b63e6d8cc52a28bccdc2c86baa390#diff-5f98ee242ca534971f89d34bc88e916059cfe3b7a9e240a843213e987f88b061L657 https://github.com/janet-lang/janet/pull/1246/commits/701913fb190b63e6d8cc52a28bccdc2c86baa390#diff-5f98ee242ca534971f89d34bc88e916059cfe3b7a9e240a843213e987f88b061L788 https://github.com/janet-lang/janet/pull/1246/commits/701913fb190b63e6d8cc52a28bccdc2c86baa390#diff-5f98ee242ca534971f89d34bc88e916059cfe3b7a9e240a843213e987f88b061L837 https://github.com/janet-lang/janet/pull/1246/commits/701913fb190b63e6d8cc52a28bccdc2c86baa390#diff-5f98ee242ca534971f89d34bc88e916059cfe3b7a9e240a843213e987f88b061L860

wooosh commented 1 year ago

They can't be made into JanetAbstract because they are const pointers. const JanetAbstract becomes the type void * const instead of the desired const void * because C's const specifier is token based. It would be nice to be able to include these, but I'm not sure how I would implement it.