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.

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

Allow windows, windows_core (and several other crates) to be compiled and used in no_std environments. #3047

Merged
merged 8 commits into from
May 20, 2024

Conversation

sivadeilra
Copy link
Collaborator

This allows many of the crates in this repo to be compiled and used in #![no_std] environments. This is a requirement for some low-level components, especially within the implementation of Windows itself.

These crates are updated: windows, windows-core, windows-registry, windows-result. All of these crates now define a new feature, std, which is enabled by default. If you disable the std feature, then the crate will be compiled with #![no_std]. Because the std features are enabled by default, existing users of these crates should not see any difference in behavior.

Nearly all of the changes are extremely simple and do not change actual codegen. These just convert std::ptr::null_mut with core::ptr::null_mut, etc.

For those cases where a crate used dynamic memory containers (Box, String, Vec), I added an explicit dependency on the alloc crate and imported the needed types. These are the same types that are re-exported from std, so this also has no effect on codegen.

In a few cases, code was using things from std that are currently only available in std, such as OsString or Mutex. In those cases, I placed the code using those items under #[cfg(feature = "std")]. Since the std feature is enabled by default, this will not break any existing users of those crates.

I also noticed a bug in passing in BasicString. It can construct a &[u16] over a null pointer, which is UB. I've fixed that bug in this PR.

crates/libs/registry/Cargo.toml Outdated Show resolved Hide resolved
@@ -11,6 +11,10 @@ readme = "readme.md"
categories = ["os::windows-apis"]
exclude = ["tests"]

[features]
default = []
std = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
std = []
std = ["windows-result/std"]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, the more I think about feature injection, I think we don't actually need it unless A-->B where A actually uses something from B's std-dependent exterior API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In which case, we should check each crate with no default features to make sure we don't miss dependencies

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • I think windows-registry could be entirely no_std.
  • I could separately add a generated yml workflow for testing no default feature builds.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I see what you mean. I'm a bit concerned at the combinatorial explosion in A{no_std?} --> B{no_std?} variations, so I'm not sure how to implement this suggestion without things blowing up. But it's definitely a valid concern...

Copy link
Collaborator

Choose a reason for hiding this comment

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

So I think the windows-registry crate doesn't need features of its own, depends on the windows-result crate with default-features = false, and can be #![no_std].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. I've removed the feature from windows-registry; it is now always no_std (except in test builds).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this has been addressed with some follow-up work. Will merge now to get this batch of changes in.

@kennykerr kennykerr merged commit 1172cf2 into microsoft:master May 20, 2024
89 checks passed
@sivadeilra sivadeilra deleted the user/ardavis/no_std branch May 23, 2024 23:05
mati865 pushed a commit to mati865/windows-rs that referenced this pull request Jun 15, 2024
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