microsoft / win32metadata

Tooling to generate metadata for Win32 APIs in the Windows SDK.
Other
1.34k stars 117 forks source link

Should `GUID_NULL` really be in `windows::Win32::Media::KernelStreaming`? #1772

Open Enyium opened 11 months ago

Enyium commented 11 months ago

Suggestion

Shell_NotifyIconGetRect() demands GUID_NULL for its struct parameter NOTIFYICONIDENTIFIER, which has nothing to to with windows::Win32::Media::KernelStreaming that GUID_NULL currently resides in.

Should a user wanting to use the shell API really have to add a dependency to this module? GUID is even from windows::core!

tim-weis commented 11 months ago

GUID::zeroed() returns the same value as GUID_NULL. You do not need to enable the "Win32_Media_KernelStreaming" feature to use this value.

Enyium commented 11 months ago

I didn't expect such a specific method, because other types always require use of default() to get an instance with zeroed memory.

This is what I mean with too large API inconsistency. When GUID_NULL exists, GUID::null() would be justified. And then, many other types would also need it, like HWND::null(), HMENU::null() etc. This, in turn, would IMO justify the trait Null that could be used for these types (and also offer is_null()).

You should be able to better get to know and predict the API, instead of being surprised by it again and again.

I also don't really like the statement that writing default() makes on types like HICON etc. For types like POINT and SIZE, depending on the context (like CreateWindowEx() with its CW_USEDEFAULT), POINT::default() and SIZE::default() can be misleading in an abstraction. And they also don't have zeroed().


Regarding GUID_NULL, I think it's still debatable whether something documented as usable for a shell function should only be available in such a different module.

riverar commented 11 months ago

This is what I mean with too large API inconsistency. When GUID_NULL exists, GUID::null() would be justified. [...]

Disagree. That would not accurately depict {00000000-0000-0000-0000-000000000000}, a valid GUID.

Regarding GUID_NULL, I think it's still debatable whether something documented as usable for a shell function should only be available in such a different module.

Fair feedback, might be something we can move around. I'll forward this issue to the win32metadata repository.

Enyium commented 11 months ago

Disagree. That would not accurately depict {00000000-0000-0000-0000-000000000000}, a valid GUID.

What's the difference? std::ptr::null() is also implemented as a function. But you could also define it as an associated constant by the trait:

#[repr(transparent)]
pub struct HWND(pub isize);

pub trait Null {
    const NULL: Self;

    fn is_null(&self) -> bool;
}

impl Null for HWND {
    const NULL: Self = HWND(0);

    fn is_null(&self) -> bool {
        self.0 == 0
    }
}

fn main() {
    let hwnd = HWND::NULL;
    println!("{}", hwnd.is_null());
}

Do you also object to GUID::NULL, even though there's GUID_NULL?