Skip to content

Add bindings for Object::SetLazyDataProperty#1972

Merged
bartlomieju merged 4 commits into
mainfrom
add-object-set-lazy-data-property
Apr 30, 2026
Merged

Add bindings for Object::SetLazyDataProperty#1972
bartlomieju merged 4 commits into
mainfrom
add-object-set-lazy-data-property

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

  • Add C++ binding and Rust FFI for v8::Object::SetLazyDataProperty
  • Expose two Rust methods: set_lazy_data_property (simple) and set_lazy_data_property_with_data (with data + attributes)
  • Add test verifying the getter fires once on first access and the property becomes a plain data property on subsequent reads

Test plan

  • CI passes (object_set_lazy_data_property test)

Expose V8's Object::SetLazyDataProperty, which sets a property whose
getter is called once on first access and then replaced with a plain
data property.
Copy link
Copy Markdown

@fibibot fibibot left a comment

Choose a reason for hiding this comment

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

Substance is right.

  • v8__Object__SetLazyDataProperty C++ binding signature matches V8's SetLazyDataProperty(Local<Context>, Local<Name>, AccessorNameGetterCallback, Local<Value> data, PropertyAttribute attr, ...). ✓
  • set_lazy_data_property / set_lazy_data_property_with_data split mirrors the existing set_accessor / set_accessor_with_data pattern in this file. ✓
  • MapFnTo<AccessorNameGetterCallback> for the getter parameter is the standard rusty_v8 pattern; getter.map_fn_to() produces the FFI-safe function pointer. ✓
  • null() for the no-data variant correctly maps to V8's Local<Value> empty case. ✓

The test is exactly the assertion I'd want — atomic counter + two evals + check the counter increments only once:

let actual = eval(scope, "obj.lazy_key").unwrap();          // first access
assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 1);
let actual2 = eval(scope, "obj.lazy_key").unwrap();         // second access
assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 1);           // not 2 ✓

This validates both halves of the contract: the getter fires on first read, and the slot is replaced with a plain data property afterward. Tight regression coverage.

Non-blocking — V8 has two more parameters

V8's full signature is:

Maybe<bool> SetLazyDataProperty(
  Local<Context>, Local<Name>, AccessorNameGetterCallback,
  Local<Value> data,
  PropertyAttribute attr,
  SideEffectType getter_side_effect_type = kHasSideEffect,
  SideEffectType setter_side_effect_type = kHasSideEffectToReceiver
);

The two SideEffectType parameters control whether DevTools' "evaluate without side effects" mode (used in console autocomplete + REPL evaluation) is allowed to invoke the getter. Defaults are sensible for most uses, so omitting them in the initial binding is fine. Worth a follow-up if a downstream consumer needs to mark a lazy getter as side-effect-free for inspector ergonomics.

CI

Mostly green; a few macos shards still pending. Already approved by @devsnek. Holding to COMMENT until the macos jobs settle, but substance is clean and I'd flip to APPROVE on green.

@bartlomieju bartlomieju merged commit 24d93d2 into main Apr 30, 2026
30 checks passed
@bartlomieju bartlomieju deleted the add-object-set-lazy-data-property branch April 30, 2026 16:57
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.

3 participants