Fix string constants for standalone sys #2500
Conversation
Jake-Shadle
force-pushed
the
fix-string-constant-type-collection
branch
from
b90b368
to
deb0645
Compare
| @@ -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 { | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. 😉
| // 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 { |
There was a problem hiding this comment.
This seems like a hack. String constants should not be depending on HSTRING.
There was a problem hiding this comment.
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.
|
I think the main issue is support for string literals in standalone code generation. I'll think some more on the best way to handle those in sys-style code generation. It should work fine for non-sys code generation. |
This change fixes two issues described in #2499.
PCSTRorPCWSTRbased on the constant field, but this was not reflected in type collection. The standalone type collection now ignores string constants since it knows they are special, soHSTRINGis no longer added if not used elsewhere.s!orw!to create the string literal, but in standalone + sys mode this means the macros are not prefixed with the crate, the only special case in the current bindgen for the crate name. Now if in standalone + sys mode it assumeswindows_coreis used and emits that.The other way to fix issue 2 is to get rid of the macros altogether in favor of just emitting either a byte string (with null terminator) for ansi, or a precomputed u16 array (with null terminator) directly in the bindings. The byte string would be as readable as the macro is today, and it would be trivial to add a comment on top of the u16 array to say the human readable string that it represents. But I did the simplest fix for now.
Resolves: #2499