From 19c6e8b6b673975d12a5c4f6b0a6d5d6c8138fc5 Mon Sep 17 00:00:00 2001 From: Osei Fortune Date: Fri, 23 Oct 2026 20:16:05 -0400 Subject: [PATCH] fix: generics closed over an enum resolve the right IID `Signature::get_string` reduced every enum to `Int32`, including inside generic type arguments, so `IAsyncOperation` was asked for the IID of `IAsyncOperation` and every member failed to construct. The same broke `IVector` and computed the wrong IID for `IReference`. Enums now keep their name inside type arguments and become `Int32` only where a value crosses the ABI: `substitute_type_vars` lowers a top-level `Var!N` enum, as do the delegate helpers and the `IReference` return path. An `IReference` parameter is boxed as a real `IReference` instead of a `PropertyValue` Int32. --- metadata/src/signature.rs | 20 ++++++++-- packages/README.md | 4 +- packages/test/enum-check.js | 50 +++++++++++++++++++++++ runtime/src/interop_test.rs | 32 +++++++++++++++ runtime/src/lib.rs | 7 ++-- runtime/src/method_call.rs | 14 +++---- runtime/src/napi_engine/invoke.rs | 1 + runtime/src/napi_engine/value.rs | 5 +++ runtime/src/property_call.rs | 39 ++++++++++++++++-- runtime/src/struct_reference.rs | 3 +- runtime/src/value.rs | 5 +++ windows-napi/test/enum-generic-test.js | 55 ++++++++++++++++++++++++++ 12 files changed, 214 insertions(+), 21 deletions(-) create mode 100644 packages/test/enum-check.js create mode 100644 windows-napi/test/enum-generic-test.js diff --git a/metadata/src/signature.rs b/metadata/src/signature.rs index cce19ff..50024c2 100644 --- a/metadata/src/signature.rs +++ b/metadata/src/signature.rs @@ -158,6 +158,17 @@ impl Signature { metadata: Option<&IMetaDataImport2>, signature: &PCCOR_SIGNATURE, preserve_arity: bool, + ) -> String { + Signature::get_string_in(metadata, signature, preserve_arity, false) + } + + /// `in_type_arg` is set inside a generic's type arguments, where an enum keeps its name: the + /// closed type's IID hashes `enum(Name;i4)`, and `Int32` there names a different type. + fn get_string_in( + metadata: Option<&IMetaDataImport2>, + signature: &PCCOR_SIGNATURE, + preserve_arity: bool, + in_type_arg: bool, ) -> String { let mut signature = signature.clone(); @@ -184,8 +195,8 @@ impl Signature { let token_type = CorTokenType(token as i32); // WinRT enums are ELEMENT_TYPE_VALUETYPE but must be passed as - // Int32 on the ABI — not as a COM pointer. - if is_enum_type(metadata_ref, token_type) { + // Int32 on the ABI, not as a COM pointer. + if !in_type_arg && is_enum_type(metadata_ref, token_type) { return "Int32".to_string(); } @@ -211,7 +222,8 @@ impl Signature { } ELEMENT_TYPE_OBJECT => "Object".to_string(), ELEMENT_TYPE_SZARRAY => { - let result = Signature::get_string(metadata, &mut signature, preserve_arity); + let result = + Signature::get_string_in(metadata, &mut signature, preserve_arity, in_type_arg); format!("{}[]", result) } ELEMENT_TYPE_VAR => { @@ -244,7 +256,7 @@ impl Signature { for i in 0..generic_arguments_count { let mut sig_type = Signature::consume_type(&mut signature); - let data = Signature::get_string(metadata, &mut sig_type, preserve_arity); + let data = Signature::get_string_in(metadata, &mut sig_type, preserve_arity, true); result += data.as_ref(); if i != generic_arguments_count.saturating_sub(1) { diff --git a/packages/README.md b/packages/README.md index ac640d6..a4279cf 100644 --- a/packages/README.md +++ b/packages/README.md @@ -98,7 +98,9 @@ nativescript-windows path/to/app.js # e.g. packages/test/test-app.js ``` `packages/test/interop-check.js` exercises the shared `NSWinRT.interop` surface (typed-value/ `reference` boxing, Pointer/OutParam, buffer + DateTime helpers) and is expected to pass on -all four engines. With a script argument the host runs it and then drives the **standalone event loop** +all four engines, as is `packages/test/enum-check.js` (generics closed over an enum: +`IAsyncOperation`, `IVector`, `IReference`). With a script argument the host +runs it and then drives the **standalone event loop** (`runtime::napi_engine::event_loop`) until the app goes idle: Windows message pump (STA WinRT async completions, delegate invokes), `setTimeout`/`setInterval` timers (`napi_engine::timers`, install-if-missing), and the engine's microtask drain. The prelude diff --git a/packages/test/enum-check.js b/packages/test/enum-check.js new file mode 100644 index 0000000..0a00fdc --- /dev/null +++ b/packages/test/enum-check.js @@ -0,0 +1,50 @@ +// Standalone-engine check for generics closed over an enum (IAsyncOperation, +// IVector, IReference): +// nativescript-windows packages/test/enum-check.js +// Prints "enum-check: N passed, M failed" once the async check settles. +'use strict'; + +var pass = 0, fail = 0; +function check(name, ok) { + if (ok) { pass += 1; } + else { fail += 1; console.error('FAIL ' + name); } +} +function attempt(name, fn) { + try { return fn(); } catch (e) { fail += 1; console.error('FAIL ' + name + ': ' + e.message); } +} + +var Enumeration = Windows.Devices.Enumeration; +attempt('IVector', function () { + var classes = new Enumeration.DevicePicker().Filter.SupportedDeviceClasses; + classes.Append(Enumeration.DeviceClass.AudioCapture); + classes.Append(Enumeration.DeviceClass.VideoCapture); + check('IVector.Size', classes.Size === 2); + check('IVector.GetAt', classes.GetAt(0) === Enumeration.DeviceClass.AudioCapture); +}); + +var Devices = Windows.Media.Devices; +attempt('IReference', function () { + var focus = new Devices.FocusSettings(); + check('IReference starts null', focus.Distance == null); + focus.Distance = Devices.ManualFocusDistance.Nearest; + check('IReference round-trips', focus.Distance === Devices.ManualFocusDistance.Nearest); + focus.Distance = null; + check('IReference clears to null', focus.Distance == null); +}); + +function done() { + console.log('enum-check: ' + pass + ' passed, ' + fail + ' failed'); +} +var op = attempt('IAsyncOperation', function () { + return Windows.Devices.Haptics.VibrationDevice.RequestAccessAsync(); +}); +if (op) { + NSWinRT.toPromise(op).then(function (status) { + check('IAsyncOperation resolves to a number', typeof status === 'number'); + check('IAsyncOperation GetResults', op.GetResults() === status); + }, function (e) { + check('IAsyncOperation resolves: ' + e.message, false); + }).then(done); +} else { + done(); +} diff --git a/runtime/src/interop_test.rs b/runtime/src/interop_test.rs index 4e292a6..ffd5233 100644 --- a/runtime/src/interop_test.rs +++ b/runtime/src/interop_test.rs @@ -2684,6 +2684,38 @@ fn ireference_parameters_and_results() { ); } +/// Generics closed over an enum (`IAsyncOperation`, `IVector`, `IReference`) +/// resolve the IID hashed from the enum's name and carry the enum as a number. +#[test] +fn enum_type_arguments() { + run_js_assert( + "enum_type_arguments", + r#" + const Enumeration = Windows.Devices.Enumeration; + const classes = new Enumeration.DevicePicker().Filter.SupportedDeviceClasses; + classes.Append(Enumeration.DeviceClass.AudioCapture); + classes.Append(Enumeration.DeviceClass.VideoCapture); + if (classes.Size !== 2) throw new Error(`Size was ${classes.Size}`); + const first = classes.GetAt(0); + if (first !== Enumeration.DeviceClass.AudioCapture) throw new Error(`GetAt(0) was ${first}`); + + const Devices = Windows.Media.Devices; + const focus = new Devices.FocusSettings(); + if (focus.Distance !== null) throw new Error(`unset Distance was ${focus.Distance}`); + focus.Distance = Devices.ManualFocusDistance.Nearest; + if (focus.Distance !== Devices.ManualFocusDistance.Nearest) throw new Error(`Distance was ${focus.Distance}`); + focus.Distance = null; + if (focus.Distance !== null) throw new Error(`cleared Distance was ${focus.Distance}`); + + const op = Windows.Devices.Haptics.VibrationDevice.RequestAccessAsync(); + return NSWinRT.toPromise(op).then((status) => { + if (typeof status !== 'number') throw new Error(`access status was ${status}`); + if (op.GetResults() !== status) throw new Error(`GetResults was ${op.GetResults()}`); + }); + "#, + ); +} + /// Struct `IReference` parameters take a plain object or a struct value, and null clears them. #[test] fn ireference_struct_parameters() { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 6fa6be6..7b19d8a 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1116,7 +1116,8 @@ pub(crate) fn classify_return(return_type: &str, is_void: bool) -> ReturnKind { return ReturnKind::Void; } if let Some(kind) = - crate::helpers::ireference_inner_type(return_type).and_then(classify_reference_return) + crate::helpers::ireference_inner_type(return_type) + .and_then(|inner| classify_reference_return(crate::property_call::abi_type_arg(inner))) { return kind; } @@ -8092,7 +8093,7 @@ pub(crate) fn delegate_param_sigs(iid_name: &str) -> Option> { .strip_prefix("Var!") .and_then(|n| n.parse::().ok()); Some(match placeholder.and_then(|n| type_args.get(n)) { - Some(arg) => arg.clone(), + Some(arg) => crate::property_call::abi_type_arg(arg).to_owned(), None => sig, }) }) @@ -8155,7 +8156,7 @@ fn delegate_return_from_invoke(method: &MethodDeclaration, type_args: &[String]) let sig = Signature::to_string(meta, &method.return_type()); let placeholder = sig.strip_prefix("Var!").and_then(|n| n.parse::().ok()); let sig = match placeholder.and_then(|n| type_args.get(n)) { - Some(arg) => arg.clone(), + Some(arg) => crate::property_call::abi_type_arg(arg).to_owned(), None => sig, }; delegate_invoke::delegate_return_for_signature(&sig, ffi_type_for_delegate_param) diff --git a/runtime/src/method_call.rs b/runtime/src/method_call.rs index 873d44a..8b15109 100644 --- a/runtime/src/method_call.rs +++ b/runtime/src/method_call.rs @@ -78,16 +78,14 @@ impl PointerPlan { type_args: &[String], typename_special: bool, ) -> Self { - if let Some(inner) = crate::helpers::ireference_inner_type(signature) { + if crate::helpers::ireference_inner_type(signature).is_some() { let name = signature.trim(); let name = name.strip_prefix("ByRef ").unwrap_or(name); - let iid = GenericInstanceIdBuilder::generate_id_from_name( - &crate::property_call::substitute_type_vars(name, type_args), - ); - return PointerPlan::IReference( - crate::property_call::substitute_type_vars(inner, type_args), - iid, - ); + // Substituted as a whole so an enum `T` keeps its name for boxing. + let name = crate::property_call::substitute_type_vars(name, type_args); + let iid = GenericInstanceIdBuilder::generate_id_from_name(&name); + let inner = crate::helpers::ireference_inner_type(&name).unwrap_or_default(); + return PointerPlan::IReference(inner.to_owned(), iid); } if !signature.contains('.') { return PointerPlan::Plain; diff --git a/runtime/src/napi_engine/invoke.rs b/runtime/src/napi_engine/invoke.rs index 3cc1106..2fb8b47 100644 --- a/runtime/src/napi_engine/invoke.rs +++ b/runtime/src/napi_engine/invoke.rs @@ -105,6 +105,7 @@ pub(crate) fn convert_call_result( } ReturnKind::Reference { value, size } => { let inner = crate::helpers::ireference_inner_type(return_type).unwrap_or(return_type); + let inner = crate::property_call::abi_type_arg(inner); return match unsafe { crate::value::read_reference_value(result, *size) } { Some(mut buf) => { let ptr = buf.as_mut_ptr() as *mut c_void; diff --git a/runtime/src/napi_engine/value.rs b/runtime/src/napi_engine/value.rs index 8dbf8e9..3f356e1 100644 --- a/runtime/src/napi_engine/value.rs +++ b/runtime/src/napi_engine/value.rs @@ -877,6 +877,11 @@ pub fn box_as_ireference( return Some((NativeValue { pointer: reference.as_raw() }, Some(reference))); } } + if vt == ValueType::Number && crate::property_call::is_enum_name(inner_type) { + let value = coerce_f64(env, arg)? as i64 as u32; + let reference = crate::struct_reference::create(value.to_le_bytes().to_vec(), *iid); + return Some((NativeValue { pointer: reference.as_raw() }, Some(reference))); + } let wrapped = if vt == ValueType::Object || vt == ValueType::Function { let obj: JsObject = unsafe { arg.cast() }; try_get_external_handle(env, &obj).filter(|ptr| !ptr.is_null()) diff --git a/runtime/src/property_call.rs b/runtime/src/property_call.rs index 0ea16a7..f6d8132 100644 --- a/runtime/src/property_call.rs +++ b/runtime/src/property_call.rs @@ -231,17 +231,48 @@ pub(crate) fn append_struct_object_bytes_napi( buf.extend(std::iter::repeat(0u8).take(tail)); } +/// Replaces `Var!N` placeholders in `s` with `type_args`. An enum argument stays named inside +/// another generic (it is part of that type's IID) and becomes its `Int32` ABI type elsewhere. pub(crate) fn substitute_type_vars(s: &str, type_args: &[String]) -> String { - if type_args.is_empty() { + if type_args.is_empty() || !s.contains("Var!") { return s.to_string(); } - let mut result = s.to_string(); - for (i, arg) in type_args.iter().enumerate() { - result = result.replace(&format!("Var!{}", i), arg.as_str()); + let mut result = String::with_capacity(s.len()); + let mut depth = 0usize; + let mut rest = s; + while let Some(c) = rest.chars().next() { + if let Some(after) = rest.strip_prefix("Var!") { + let digits = after.bytes().take_while(u8::is_ascii_digit).count(); + if let Some(arg) = after[..digits].parse::().ok().and_then(|i| type_args.get(i)) { + result.push_str(if depth == 0 { abi_type_arg(arg) } else { arg }); + rest = &after[digits..]; + continue; + } + } + match c { + '<' => depth += 1, + '>' => depth = depth.saturating_sub(1), + _ => {} + } + result.push(c); + rest = &rest[c.len_utf8()..]; } result } +/// The ABI type of a generic type argument: `Int32` for an enum, the argument itself otherwise. +pub(crate) fn abi_type_arg(arg: &str) -> &str { + if is_enum_name(arg) { "Int32" } else { arg } +} + +/// Whether `name` is a WinRT enum's full name. +pub(crate) fn is_enum_name(name: &str) -> bool { + name.contains('.') + && !name.contains('<') + && MetadataReader::find_by_name(name) + .is_some_and(|decl| matches!(decl.read().kind(), DeclarationKind::Enum)) +} + /// Immutable per-property-method-type data cached after first construction. /// Class path key: (method_token as u64) << 1 | (is_initializer as u64). /// Interface path key: (token, declaring IID, is_setter) — the IID disambiguates diff --git a/runtime/src/struct_reference.rs b/runtime/src/struct_reference.rs index 24c5e0c..2d86d3b 100644 --- a/runtime/src/struct_reference.rs +++ b/runtime/src/struct_reference.rs @@ -3,7 +3,8 @@ //! //! `PropertyValue` only boxes primitives and a few Foundation types, so the struct's bytes (laid //! out as for a struct parameter) are held by an agile object that answers the reference's IID, -//! which is computed from its closed generic name, and copies them out of `get_Value`. +//! which is computed from its closed generic name, and copies them out of `get_Value`. An enum +//! `IReference` holds the enum's 4 bytes the same way. use std::ffi::c_void; use std::sync::atomic::{AtomicU32, Ordering}; diff --git a/runtime/src/value.rs b/runtime/src/value.rs index 2971507..4146d68 100644 --- a/runtime/src/value.rs +++ b/runtime/src/value.rs @@ -1111,6 +1111,11 @@ pub fn box_as_ireference( return Some((NativeValue { pointer: reference.as_raw() }, Some(reference))); } } + if arg.is_number() && crate::property_call::is_enum_name(inner_type) { + let value = arg.number_value(scope)? as i64 as u32; + let reference = crate::struct_reference::create(value.to_le_bytes().to_vec(), *iid); + return Some((NativeValue { pointer: reference.as_raw() }, Some(reference))); + } let wrapped = if arg.is_object() { arg.to_object(scope) .and_then(|obj| try_get_external_handle(scope, obj)) diff --git a/windows-napi/test/enum-generic-test.js b/windows-napi/test/enum-generic-test.js new file mode 100644 index 0000000..864848b --- /dev/null +++ b/windows-napi/test/enum-generic-test.js @@ -0,0 +1,55 @@ +// Generics closed over an enum: IAsyncOperation, IVector and IReference. The +// closed type's IID hashes the enum's name, so the enum must not be reduced to Int32 there. +const { Windows, toPromise, awaitWithPump } = require('../nswinrt.js'); + +let pass = 0, fail = 0; +function check(name, cond) { + if (cond) pass++; + else { console.log(`FAIL ${name}`); fail++; } +} +function attempt(name, fn) { + try { return fn(); } catch (e) { console.log(`FAIL ${name}: ${e.message}`); fail++; } +} + +async function main() { + // IAsyncOperation + const Haptics = Windows.Devices.Haptics; + const op = Haptics.VibrationDevice.RequestAccessAsync(); + check('IAsyncOperation exposes Status', typeof op.Status === 'number'); + let status; + try { + status = await awaitWithPump(toPromise(op), 5000); + } catch (e) { + console.log(`FAIL toPromise(IAsyncOperation): ${e.message}`); + fail++; + } + check('IAsyncOperation resolves to a number', typeof status === 'number'); + check('IAsyncOperation GetResults after completion', attempt('GetResults', () => op.GetResults()) === status); + + // IVector + const Enumeration = Windows.Devices.Enumeration; + const classes = attempt('IVector', () => new Enumeration.DevicePicker().Filter.SupportedDeviceClasses); + if (classes) { + attempt('IVector.Append', () => { + classes.Append(Enumeration.DeviceClass.AudioCapture); + classes.Append(Enumeration.DeviceClass.VideoCapture); + }); + check('IVector.Size', classes.Size === 2); + check('IVector.GetAt', attempt('GetAt', () => classes.GetAt(0)) === Enumeration.DeviceClass.AudioCapture); + check('IVector[i]', classes[1] === Enumeration.DeviceClass.VideoCapture); + } + + // IReference + const Devices = Windows.Media.Devices; + const focus = new Devices.FocusSettings(); + check('IReference starts null', focus.Distance == null); + attempt('IReference set', () => { focus.Distance = Devices.ManualFocusDistance.Nearest; }); + check('IReference round-trips', focus.Distance === Devices.ManualFocusDistance.Nearest); + attempt('IReference set null', () => { focus.Distance = null; }); + check('IReference clears to null', focus.Distance == null); + + console.log(`\n${pass} passed, ${fail} failed`); + process.exitCode = fail ? 1 : 0; +} + +main().catch((e) => { console.log('FATAL', e && e.stack || e); process.exitCode = 1; });