-
-
Notifications
You must be signed in to change notification settings - Fork 159
fix(hir,runtime): native chained-static-class decorators + reflect-metadata require binding (NestJS bootstrap) #5721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -30,6 +30,73 @@ pub extern "C" fn js_object_get_field_by_name( | |||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| // A per-evaluation class object (`ClassExprFresh`, #1772/#1787) reaches | ||||||||||||||||
| // here as a RAW heap pointer (a real ObjectHeader, so its top 16 address | ||||||||||||||||
| // bits are 0 — distinguishing it from a `0x7FFE` class-ref value or any | ||||||||||||||||
| // NaN-boxed value). Its static METHODS / static ACCESSORS live in the class | ||||||||||||||||
| // registry keyed by the header class_id, never as own properties, so a read | ||||||||||||||||
| // like `C.staticMethod` returned `undefined` (the class-ref form resolves | ||||||||||||||||
| // these via the registry; this pointer-tagged class-object form did not). | ||||||||||||||||
| // That is NestJS's `Logger.error` when the Logger takes the fresh path | ||||||||||||||||
| // (captures `DEFAULT_LOGGER`), which the tslib `__decorate` chain then reads | ||||||||||||||||
| // `.value` off → "reading 'value'". Resolve own fields first (own-property | ||||||||||||||||
| // precedence), then fall back to the registry. The `(obj >> 48) == 0` guard | ||||||||||||||||
| // ensures `is_class_object_ptr` only ever sees a real heap pointer (it | ||||||||||||||||
| // back-reads a GcHeader), never a tagged value — which previously SIGSEGV'd. | ||||||||||||||||
| if !key.is_null() | ||||||||||||||||
| && ((obj as u64) >> 48) == 0 | ||||||||||||||||
| // Must be ABOVE the whole small-handle band (>= 0x100000), not just | ||||||||||||||||
| // >= 0x10000: native handle ids in [0x10000, 0x100000) (fetch/http/…) | ||||||||||||||||
| // would otherwise reach `is_class_object_ptr`, which back-reads a | ||||||||||||||||
| // GcHeader and SIGSEGVs on the non-heap handle id. | ||||||||||||||||
| && crate::value::addr_class::is_above_handle_band(obj as usize) | ||||||||||||||||
| && crate::object::class_registry::is_class_object_ptr(obj as *const u8) | ||||||||||||||||
| { | ||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||
| let own = get_field_by_name_object_tail(obj, key); | ||||||||||||||||
| if !own.is_undefined() { | ||||||||||||||||
| return own; | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+55
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Use an own-property check before falling back to the class registry.
Suggested fix- let own = get_field_by_name_object_tail(obj, key);
- if !own.is_undefined() {
- return own;
+ if super::super::own_key_present(obj, key) {
+ return get_field_by_name_object_tail(obj, key);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| unsafe { | ||||||||||||||||
| // Re-box the raw class-object pointer as a POINTER-tagged JS value | ||||||||||||||||
| // so `js_class_method_bind` (which expects a value, like the | ||||||||||||||||
| // class-ref path) binds the static method to the right receiver. | ||||||||||||||||
| let class_value = f64::from_bits(crate::value::js_nanbox_pointer(obj as i64).to_bits()); | ||||||||||||||||
| let class_id = super::super::js_object_get_class_id(obj); | ||||||||||||||||
| if class_id != 0 { | ||||||||||||||||
| let name_ptr = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>()); | ||||||||||||||||
| let name_len = (*key).byte_len as usize; | ||||||||||||||||
| let name = std::str::from_utf8(std::slice::from_raw_parts(name_ptr, name_len)) | ||||||||||||||||
| .unwrap_or(""); | ||||||||||||||||
| if !name.is_empty() | ||||||||||||||||
| && !super::super::class_registry::class_is_key_deleted(class_id, name) | ||||||||||||||||
| { | ||||||||||||||||
| if super::super::class_registry::lookup_static_method_in_chain(class_id, name) | ||||||||||||||||
| .is_some() | ||||||||||||||||
| { | ||||||||||||||||
| let heap_name = { | ||||||||||||||||
| let layout = | ||||||||||||||||
| std::alloc::Layout::from_size_align(name_len.max(1), 1).unwrap(); | ||||||||||||||||
| let ptr = std::alloc::alloc(layout); | ||||||||||||||||
| std::ptr::copy_nonoverlapping(name_ptr, ptr, name_len); | ||||||||||||||||
| ptr | ||||||||||||||||
| }; | ||||||||||||||||
| let result = js_class_method_bind(class_value, heap_name, name_len); | ||||||||||||||||
| return JSValue::from_bits(result.to_bits()); | ||||||||||||||||
| } | ||||||||||||||||
| if let Some(v) = | ||||||||||||||||
| super::super::class_registry::class_static_accessor_getter_value( | ||||||||||||||||
| class_id, | ||||||||||||||||
| name, | ||||||||||||||||
| class_value, | ||||||||||||||||
| ) | ||||||||||||||||
| { | ||||||||||||||||
| return JSValue::from_bits(v.to_bits()); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| return own; | ||||||||||||||||
| } | ||||||||||||||||
| if let Some(addr) = | ||||||||||||||||
| crate::typedarray_props::typed_array_addr_from_value(f64::from_bits(obj as u64)) | ||||||||||||||||
| { | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Mirror static accessor descriptors for fresh class objects.
The fresh-class branch only reports static methods.
Object.getOwnPropertyDescriptor(C, "x")forstatic get x()will still fall through toundefined, while the class-ref path below already returns accessor descriptors.Suggested fix
if class_id != 0 && !super::class_registry::class_is_key_deleted(class_id, &method_name) - && super::class_registry::class_has_own_static_method( - class_id, - &method_name, - ) { + if let Some((g, s)) = + super::class_registry::class_own_static_accessor_ptrs( + class_id, + &method_name, + ) + { + return build_accessor_descriptor( + super::class_registry::class_accessor_function_value( + g, + false, + &method_name, + ), + super::class_registry::class_accessor_function_value( + s, + true, + &method_name, + ), + false, + true, + ); + } + if !super::class_registry::class_has_own_static_method( + class_id, + &method_name, + ) { + return f64::from_bits(crate::value::TAG_UNDEFINED); + } let leaked: &'static [u8] = method_name.as_bytes().to_vec().leak(); let value = super::js_class_method_bind( obj_value,📝 Committable suggestion
🤖 Prompt for AI Agents