Skip to content

lunatik: replace bool flags with u8 bitmask in class and object#477

Closed
carloslack wants to merge 3 commits intomasterfrom
claude_flags
Closed

lunatik: replace bool flags with u8 bitmask in class and object#477
carloslack wants to merge 3 commits intomasterfrom
claude_flags

Conversation

@carloslack
Copy link
Collaborator

Replace the separate bool fields (.sleep, .shared, .pointer) in lunatik_class_t and lunatik_object_t with a single u8 flags field using named bitmask constants: LUNATIK_SLEEPABLE, LUNATIK_SHARABLE, and LUNATIK_EXTERNAL.

This eliminates ambiguity (same field name, different semantics in class vs object), simplifies function signatures (lunatik_newobject, lunatik_createobject, lunatik_runtime each lose a parameter), and makes call sites self-documenting.

kovid-labs and others added 2 commits March 10, 2026 14:10
Replace the separate bool fields (.sleep, .shared, .pointer) in
lunatik_class_t and lunatik_object_t with a single u8 flags field
using named bitmask constants: LUNATIK_SLEEPABLE, LUNATIK_SHARABLE,
and LUNATIK_EXTERNAL.

This eliminates ambiguity (same field name, different semantics in
class vs object), simplifies function signatures (lunatik_newobject,
lunatik_createobject, lunatik_runtime each lose a parameter), and
makes call sites self-documenting.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
lunatik_setclass() now takes u8 flags, so the call in lunatik_lruntime
was passing true (=1) instead of LUNATIK_SHARABLE (=2), silently
registering the runtime as non-shared. Fix that and align the
continuation backslashes in lunatik_locker and LUNATIK_NEWLIB to match
the surrounding macros.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
lunatik_obj.c Outdated
if (release)
release(private);
if (!class->pointer)
if (!(class->flags & LUNATIK_EXTERNAL))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lneto do you agree with this change? I think external is more contextual meaning than pointer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

lib/luaprobe.c Outdated
static int luaprobe_new(lua_State *L)
{
lunatik_object_t *object = lunatik_newobject(L, &luaprobe_class, sizeof(luaprobe_t), false);
lunatik_object_t *object = lunatik_newobject(L, &luaprobe_class, sizeof(luaprobe_t), 0);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept 0 because it is universal , adding a LUNATIK_NONE would be just noise IMO

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed feelings.. I like NONE, but it's your call.. ;-)

.release = luacrypto_rng_release,
.sleep = true,
.shared = true,
.pointer = true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I like, because we finally can kill those variables in favour of a single one with meaningful flag names

@carloslack carloslack requested a review from lneto March 10, 2026 14:39
.sleep = true,
.shared = true,
.pointer = true,
.flags = LUNATIK_SLEEPABLE | LUNATIK_SHARABLE | LUNATIK_EXTERNAL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we use this case a lot, perhaps we can define an alias.. LUNATIK_DEFAULT?

lib/luafib.c Outdated

static const lunatik_class_t luafib_class = {
.sleep = true,
.flags = LUNATIK_SLEEPABLE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to have a class without mt, I think it may be a leftover and we should get rid of class completely here..

lunatik.h Outdated
spin_op(&(o)->spin); \
#define LUNATIK_SLEEPABLE (1U << 0)
#define LUNATIK_SHARABLE (1U << 1)
#define LUNATIK_EXTERNAL (1U << 2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should prefix them with LUNATIK_OPT or LUNATIK_FLAG, what do you think? It might be more verbose, but the namespace will be cleaner, I think

lunatik.h Outdated
bool sleep;
bool shared;
bool pointer;
u8 flags; /* LUNATIK_SLEEPABLE | LUNATIK_SHARABLE | LUNATIK_EXTERNAL */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed feelings about this comment..

@lneto lneto requested a review from sneaky-potato March 10, 2026 16:21
@lneto lneto marked this pull request as ready for review March 10, 2026 22:02
- Add LUNATIK_FLAG_ prefix to flag constants (SLEEPABLE, SHARABLE, EXTERNAL)
  to distinguish them from other LUNATIK_ macros
- Add LUNATIK_DEFAULT alias for the common SLEEPABLE|SHARABLE|EXTERNAL combo
  used by most class definitions
- Remove inline comments from u8 flags fields in class/object structs
- Add LUNATIK_FLAG_NONE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants