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

Feature request: Provide more const support for string types #2049

Closed
seritools opened this issue Sep 21, 2022 · 5 comments · Fixed by #2078
Closed

Feature request: Provide more const support for string types #2049

seritools opened this issue Sep 21, 2022 · 5 comments · Fixed by #2078
Labels
enhancement New feature or request

Comments

@seritools
Copy link
Contributor

seritools commented Sep 21, 2022

Motivation

w! allows you to create a &'static HSTRING, but .as_ptr() and other conversions don't seem to be const, so it doesn't seem possible to get a const PCWSTR from it, in const context. Currently I'm using wchar::wchz! as alternative: PCWSTR::from_raw(wchz!("my string").as_ptr()). It would be nice to have a built-in way to do this.

Another tiny one is to create a const null-PCWSTR, but PCWSTR::null is not marked as const. PCWSTR(std::ptr::null()) is the slightly uglier workaround :)
The same should be the case for the other string types (PWSTR, and the 8bit string types). null(), as_ptr() and is_null() all can be marked as const fn as far as I can tell.

Drawbacks

No response

Rationale and alternatives

No response

Additional context

No response

@seritools seritools added the enhancement New feature or request label Sep 21, 2022
@kennykerr
Copy link
Collaborator

I'd love to get there. I think we're still waiting for things to stabilize. For example, if I try to make HSTRING::is_empty a const function I get the following error:

D:\git\windows-rs>cargo check -p windows
    Checking windows v0.40.0 (D:\git\windows-rs\crates\libs\windows)
    Finished dev [unoptimized + debuginfo] target(s) in 0.55s

D:\git\windows-rs>cargo check -p windows
    Checking windows v0.40.0 (D:\git\windows-rs\crates\libs\windows)
error: `ptr::mut_ptr::<impl *mut T>::is_null` is not yet stable as a const fn
  --> crates\libs\windows\src\core\strings\hstring.rs:19:9
   |
19 |         self.0.is_null()
   |         ^^^^^^^^^^^^^^^^
   |
   = help: add `#![feature(const_ptr_is_null)]` to the crate attributes to enable

error: could not compile `windows` due to previous error

I'm not sure when the const_ptr_is_null feature will be stabilized. It doesn't seem to have made much progress of late:

rust-lang/rust#74939

@ChrisDenton
Copy link
Collaborator

Hm, if you don't mind a bit of type verbosity you maybe could use Option<NonNull<Header>> as the inner type. Then it's just None for the null case.

@seritools
Copy link
Contributor Author

seritools commented Sep 22, 2022

Testing a bit on HSTRING (https://github.com/seritools/windows-rs/tree/hstring-tests) it seems that it's possible.

There's some ugliness around NonNull::as_ptr() returning a *mut, which rustc currently doesn't allow to deref in const context, triggering:

error[E0658]: dereferencing raw mutable pointers in constant functions is unstable
  --> crates\libs\windows\src\core\strings\hstring.rs:40:22
   |
40 |             unsafe { (*header).data }
   |                      ^^^^^^^^^^^^^^
   |
   = note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
   = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable

even if the pointer is only read, but manually casting it to *const Header seems to resolve it.

With Rust 1.64 released today even as_wide() can be marked as const, as core::slice::from_raw_parts is now a const fn :)

@kennykerr
Copy link
Collaborator

Thanks, that seems to work:

https://github.com/microsoft/windows-rs/compare/const-hstring?expand=1

It just breaks the debug visualizer test for some reason. I'll try to figure that out and then get a PR up.

cc @ridwanabdillahi

@ridwanabdillahi
Copy link
Contributor

@kennykerr I'll take a look at the failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants