Skip to content
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.

Sign up for GitHub

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

Fix string constants for standalone sys #2500

Closed
wants to merge 1 commit into from
Closed

Fix string constants for standalone sys #2500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion crates/libs/bindgen/src/constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ pub fn gen(gen: &Gen, def: Field) -> TokenStream {

if ty == constant_type {
if ty == Type::String {
let crate_name = gen.crate_name();
let crate_name: TokenStream = if gen.sys && !gen.standalone {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bit problematic since this assumes that sys-style code gen should assume a dependency on windows-core which is designed for non-sys style code gen. I'd like to avoid adding a "core" crate for windows-sys but this conflates the two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I considered this the simpler fix in hopes of merging, but like I said in the PR description I think the better option is just to remove the need for the macros altogether, but I figured that would be harder to pass review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, it's not a simple fix but I'd prefer to find a good solution rather than fix a problem by introducing another problem. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

"::windows_sys::core::".into()
} else {
"::windows_core::".into()
};
if gen.reader.field_is_ansi(def) {
let value = gen.value(&gen.reader.constant_value(constant));
quote! {
Expand Down
11 changes: 9 additions & 2 deletions crates/libs/bindgen/src/standalone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,15 @@ fn standalone_imp(gen: &mut Gen, names: &[&str]) -> String {
.find(|field| gen.reader.field_name(*field) == type_name.name)
{
constants.insert(field);
gen.reader
.type_collect_standalone(&gen.reader.field_type(field, None), &mut types);

let ft = gen.reader.field_type(field, None);

// String normally gets resolved to HSTRING, but since macros
// from windows_core/windows_sys::core are used to create the
// string literals we can skip the type collection
if ft != Type::String {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like a hack. String constants should not be depending on HSTRING.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that they are in fact System.String types while windows-rs assumes that is always an HSTRING. That's a separate issue but I have a plan to address that.

gen.reader.type_collect_standalone(&ft, &mut types);
}
}

if let Some(field) = gen
Expand Down
13 changes: 12 additions & 1 deletion crates/tests/standalone/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,18 @@ fn main() {
write_sys(
"src/b_overloads.rs",
&["Windows.Win32.NetworkManagement.NetManagement.AE_RESACCESS"],
)
);

// Ensures that string constants are written correctly
write_sys(
"src/b_string_constants.rs",
&[
// utf-16
"Windows.Win32.Devices.Enumeration.Pnp.ADDRESS_FAMILY_VALUE_NAME",
// ansi
"Windows.Win32.Media.Multimedia.JOY_CONFIGCHANGED_MSGSTRING",
],
);
}

fn write_sys(filename: &str, apis: &[&str]) {
Expand Down
14 changes: 14 additions & 0 deletions crates/tests/standalone/src/b_string_constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Bindings generated by `windows-bindgen` 0.49.0

#![allow(
non_snake_case,
non_upper_case_globals,
non_camel_case_types,
dead_code,
clippy::all
)]
pub const ADDRESS_FAMILY_VALUE_NAME: ::windows_core::PCWSTR = ::windows_core::w!("AddressFamily");
pub const JOY_CONFIGCHANGED_MSGSTRING: ::windows_core::PCSTR =
::windows_core::s!("MSJSTICK_VJOYD_MSGSTR");
pub type PCSTR = *const u8;
pub type PCWSTR = *const u16;
1 change: 1 addition & 0 deletions crates/tests/standalone/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod b_pcwstr;
mod b_pstr;
mod b_pwstr;
mod b_std;
mod b_string_constants;
mod b_stringable;
mod b_test;
mod b_unknown;
Expand Down