Skip to content

Using robots to fix/update docs part 6: cppwinrt* headers#657

Open
dunhor wants to merge 1 commit into
masterfrom
dunhor/cppwinrt_docs
Open

Using robots to fix/update docs part 6: cppwinrt* headers#657
dunhor wants to merge 1 commit into
masterfrom
dunhor/cppwinrt_docs

Conversation

@dunhor

@dunhor dunhor commented Jul 22, 2026

Copy link
Copy Markdown
Member

See #652 for more info on the effort.

Figured that I should probably group all the C++/WinRT headers into a single change. This is another one Copilot had a bit of trouble producing good/correct documentation to & required some TLC.

Additionally, while doing this I noticed a number of potential issues. I'll note them in comments as follow-ups; I want to leave these changes as doc updates only.

Comment thread include/wil/cppwinrt.h
Comment thread include/wil/cppwinrt.h
* @def INIT_NOTIFYING_PROPERTY
* @brief use this to initialize a wil::single_threaded_notifying_property in your class constructor.
*/
#define INIT_NOTIFYING_PROPERTY(NAME, VALUE) NAME(&m_propertyChanged, *this, L"" #NAME, VALUE)

@dunhor dunhor Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This should be this->m_propertyChanged because the member comes from notify_property_changed_base, however that type uses CRTP and if the derived type is itself a template, then the base becomes dependent and this will fail under proper two phase name lookup.

Comment thread include/wil/cppwinrt.h
//! This overload is found by the `RETURN_IF_FAILED` / `THROW_IF_FAILED` family (and friends) so a `winrt::hresult` result can be
//! checked directly, without first converting it to an `HRESULT`.
//! @param hr The `winrt::hresult` to validate.
//! @return `hr`, as an `HRESULT`-compatible value.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a lot of documentation for a function people aren't expected to call directly.
Maybe just

//! Adds `winrt::hresult` support to the `verify_hresult` helper function.
//! This allows macros like `RETURN_IF_FAILED` to be used with `winrt::hresult`.

Comment thread include/wil/cppwinrt.h
//! @tparam T A C++/WinRT projected type.
//! @param object The object whose ABI representation to return.
//! @return For an interface or runtime class, a borrowed (not AddRef'd) `void*` interface pointer; for a value type, the
//! `winrt::impl::abi_t<T>` ABI struct by value.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Too much inside baseball. How about

//! Returns the raw ABI representation of a C++/WinRT projected object.
//! This is an improvement over `winrt::get_abi` because it returns `HSTRING`
//! rather than `void*` when passed a `winrt::hstring`.

Comment thread include/wil/cppwinrt.h
return winrt::get_abi(object);
}

//! Returns the raw `HSTRING` that backs a `winrt::hstring`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

//! Overload of `wil::get_abi` for `winrt::hstring` which returns the underlying ABI `HSTRING`.

Comment thread include/wil/cppwinrt.h
return static_cast<HSTRING>(winrt::get_abi(object));
}

//! Returns a null-terminated wide-character pointer to a `winrt::hstring`'s buffer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

//! Adds `winrt::hstring` support to the `str_raw_ptr` function.
//! See `str_raw_ptr` for details.

Comment thread include/wil/cppwinrt.h
return str.c_str();
}

//! Returns the address of a C++/WinRT object's ABI pointer, for receiving it as an out-parameter in generic code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, talking too much.

//! Returns the address of a C++/WinRT object's ABI pointer, for receiving it as an out-parameter.
//! This is an improvement over `winrt::get_abi` because it returns a pointer to `HSTRING`
//! rather than a pointer to `void*` when passed a `winrt::hstring`.

Comment thread include/wil/cppwinrt.h
Comment on lines +400 to +401
Queries `obj` for `Interface`, invokes `method` with `args`, and wraps the result as a `WinRTResult`. Use this with per-instance
interop interfaces such as `IUserActivityInterop`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Queries `obj` for `Interface`, invokes `method` with `args`, and wraps the result as a `WinRTResult`. Use this with per-instance
interop interfaces such as `IUserActivityInterop`.
Use this with per-instance interop interfaces such as `IUserActivityInterop`.

Comment thread include/wil/cppwinrt.h
++winrt::get_module_lock();
}

//! Takes an additional reference on the host module; every live copy holds its own lock.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
//! Takes an additional reference on the host module; every live copy holds its own lock.
//! Takes a reference on the host C++/WinRT, module, incrementing its lock count.

Comment thread include/wil/cppwinrt.h
{
using Implements::Implements;

//! Overrides C++/WinRT's interface lookup so conditionally-implemented interfaces are hidden when disabled.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This method is not meant to be called by clients. Should remain an internal method, not a documented one.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Consequence of using "no warnings" as a termination condition. I should let it be more aggressive when adding @cond/@endcond blocks

`CoRegisterClassObject`, or a `DllGetClassObject` implementation); @ref register_com_server wraps the `CoRegisterClassObject` case
for you.
@tparam T The C++/WinRT class the factory creates.
@tparam Rest Additional C++/WinRT `implements` markers for the factory (not interfaces to implement), appended to its

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not interfaces for the factory to implement?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

While this type is not final, it doesn't seem like the intent is for consumers to derive from it (which would be required if they were to list interfaces here). I suppose they could supply types that implement the interface methods, but then the comment would still technically be correct.

I can augment this comment to just say it's "typically" used for markers and keep it intentionally semi-vague.

//! @param outer The aggregating object's controlling `IUnknown`; must be null, since aggregation is unsupported.
//! @param iid Interface to return in `result`.
//! @param result Receives the requested interface on success, or null on failure.
//! @return `S_OK` on success; `CLASS_E_NOAGGREGATION` if `outer` is non-null; otherwise a failure `HRESULT`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't document this publicly. Clients do not call it directly. This is implementing a method that is called by COM itself.

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.

2 participants