diff --git a/.github/skills/doxygen-comments/SKILL.md b/.github/skills/doxygen-comments/SKILL.md index 3b27ef9b..f3494659 100644 --- a/.github/skills/doxygen-comments/SKILL.md +++ b/.github/skills/doxygen-comments/SKILL.md @@ -37,13 +37,18 @@ Do **not** rewrite comments purely for style, and do not document private/intern - **Brief = first sentence, on its own line.** `JAVADOC_AUTOBRIEF` makes the first sentence (up to the first period) the brief. Keep it on its own physical line; start the detailed description on the next line. An explicit `@brief` is rarely needed. - **Wrap at 130 columns.** This matches `.clang-format` (`ColumnLimit: 130`) and applies to comment text too. -- **Public API only.** `EXTRACT_ALL = NO` and `EXTRACT_PRIVATE = NO`, so private members and undocumented internals are not - emitted. Focus documentation on the public surface; don't add Doxygen tags to private helpers expecting them to appear in the - output. +- **Public API only — but protected members still emit.** `EXTRACT_ALL = NO` and `EXTRACT_PRIVATE = NO`, so private members and + undocumented internals are not emitted. Focus documentation on the public surface; don't add Doxygen tags to private helpers + expecting them to appear in the output. Note that `EXTRACT_PROTECTED` is **not** set in `docs/Doxyfile`, so it defaults to + `YES` — protected members *are* emitted and *do* warn when undocumented. Either document a protected member or wrap just the + protected section in `/// @cond` … `/// @endcond`; private members need no such guard. - **Hide internal `details` namespaces.** WIL's implementation details live in namespaces named `details` (and similarly named variants such as `details_abi`). These must not emit documentation — wrap the entire namespace in a `/// @cond` … `/// @endcond` pair (note the `///` marker used for these structural tags) so Doxygen skips its contents. Put `/// @cond` on its own line - immediately before the namespace and `/// @endcond` immediately after its closing brace. See the example below. + immediately before the namespace and `/// @endcond` immediately after its closing brace. See the example below. **Exception:** + occasionally a `details` type is the natural host for members shared by public types (e.g. a common base whose methods the + public types inherit). You may document that one type even though it lives in `details` — add a `@note` stating it is internal + and not for direct use, and still `/// @cond` its own protected/private plumbing so only the shared public members show. - **Add a usage example when the call pattern is non-obvious.** Include a short `~~~` fenced example for functions whose correct use isn't clear from the signature alone — e.g. callback or functor contracts (what the callback must do and return), paired or multi-step call sequences, RAII helpers whose placement or lifetime matters, round-trip or reverse operations, or subtle @@ -51,7 +56,10 @@ Do **not** rewrite comments purely for style, and do not document private/intern Doxygen fenced code blocks are delimited with `~~~` (any matching run of three or more tildes); inside `//!` banners, prefix each example line with `//!`. - **Cross-references.** Link to other entities with `@ref ` and `@see`, and group related members with `@ingroup ` - (for example `@ingroup outparam`). + (for example `@ingroup outparam`). Prefer `@ref func()` (with the parentheses) when linking a function — the `()` delimits the + name, so a trailing comma, period, or closing paren won't break the link, unlike a bare `@ref name`. Backticked code such as + `get()` never becomes a link; use `@ref get()` when you want one. A `@ref` on a derived type resolves members it inherits from + a base. - **Namespaces.** Public entities live under `wil::`; STL-mirroring pieces live under `wistd::`. - **Prefer a `PREDEFINED` macro over a per-guard escape; use `WIL_DOXYGEN` only when needed.** Doxygen evaluates `#if` guards against `docs/Doxyfile`'s `PREDEFINED` list, which already forces many conditions true in docs — e.g. @@ -65,6 +73,18 @@ Do **not** rewrite comments purely for style, and do not document private/intern - **Macros are expanded for docs.** Several macros are expanded when generating documentation (`WI_NOEXCEPT` → `noexcept`, `WI_NODISCARD` → `[[nodiscard]]`, and others in `docs/Doxyfile`'s `PREDEFINED`), so document the logical signature rather than the macro-heavy source. +- **Deleted members render but never warn.** Doxygen emits `= delete`d member functions (they show as a row in the output) but + does *not* warn when they are undocumented — so "deleted means hidden" is false. Document a deleted overload when the deletion + itself is the contract (e.g. an `operator co_await() &` deleted to reject awaiting an lvalue); otherwise it appears as a bare + `= delete` with no explanation. +- **Keep every comment self-contained.** Doxygen reorders members (alphabetically) in the output, so a comment must stand on its + own: never refer to declaration order ("the previous overload", "as above") or write "behaves like X, except…". Restate the + relevant behavior on each entity even if it repeats a sibling. +- **Backtick `#include`s and header names in comment text.** A bare `#include` in a comment triggers an "explicit link request to + 'include' could not be resolved" warning, and a bare `` is parsed as an HTML tag and dropped from the output. Write the + whole thing as code: `#include ` and ``. +- **Keep comment text ASCII.** Don't paste em dashes (—), ellipses (…), smart quotes, or arrows into `.h` comment text; use `-`, + `;`, `:`, `...`, or plain quotes. (This applies to the header comments only — Markdown docs like this file may use them.) ## Correction checklist @@ -80,13 +100,18 @@ When adding or fixing comments, verify: 6. **Cross-references resolve** — `@ref`/`@see` targets exist and are spelled correctly. 7. **The description still matches behavior** after any signature or behavior change. 8. **Lines wrap at 130 columns** and the comment marker style matches the surrounding code. -9. **No documentation is added to private/internal members** unless they are intentionally part of the documented surface. +9. **No documentation is added to private members**, but **protected members are documented or `@cond`-hidden** (they emit and + warn by default), unless intentionally part of the documented surface. 10. **Internal `details`/`details_*` namespaces are wrapped in `/// @cond` … `/// @endcond`** so their contents are excluded from the generated documentation. 11. **Conditionally-compiled public declarations are visible in docs** — the guard is either already satisfied by `PREDEFINED` or ORs in `|| defined(WIL_DOXYGEN)` (reserved for conditions `PREDEFINED` can't cover, like the header-wrapper guards). 12. **Non-obvious functions carry a `~~~` usage example** — anything with a callback contract, a paired/multi-step call sequence, or subtle ownership/buffer semantics shows how to call it; trivial helpers do not. +13. **Comment text is plain ASCII and `#include`s/header names are backticked** — no em dashes, ellipses, or smart quotes in + header comments, and `#include ` / `` are written as code spans so Doxygen doesn't mangle them. +14. **Each comment stands on its own** — no references to declaration order or "same as the previous overload", since Doxygen + reorders members in the output. ## Validating changes diff --git a/include/wil/coroutine.h b/include/wil/coroutine.h index 7381fd63..5de6496a 100644 --- a/include/wil/coroutine.h +++ b/include/wil/coroutine.h @@ -19,7 +19,7 @@ * - T must be a copyable object, movable object, reference, or void. * - The coroutine may be awaited at most once. The second await will crash. * - The coroutine may be abandoned (allowed to destruct without co_await), - * in which case unobserved exceptions are fatal. + * in which case unobserved exceptions are ignored. * - By default, wil::task resumes on an arbitrary thread. * - By default, wil::com_task resumes in the same COM apartment. * - task.resume_any_thread() allows resumption on any thread. @@ -188,9 +188,9 @@ template struct com_task; } // namespace wil -/// @cond namespace wil::details::coro { +/// @cond // task and com_task are convertible to each other. However, not // all consumers of this header have COM enabled. Support for saving // COM thread-local error information and restoring it on the resuming @@ -598,18 +598,32 @@ struct agile_awaiter return promise->client_await_resume(); } }; +/// @endcond +/** Shared awaiter interface inherited by @ref wil::task and @ref wil::com_task. +You never name or instantiate this type directly; it exists solely so that both task types share the same interface. +@note This type exists in the `wil::details` namespace and is therefore not meant for explicit consumption. It is documented for + the sole purpose of providing documentation for the various public members exposed by the task types. */ template struct task_base { + //! Returns an awaitable that resumes the caller on an arbitrary thread. + //! This matches @ref wil::task's default, so it is used mainly on a @ref wil::com_task to override same-apartment resumption + //! for a single `co_await`. + //! @return An awaitable for the task's result that completes on an arbitrary thread. auto resume_any_thread() && noexcept { return agile_awaiter{wistd::move(promise)}; } - // You must #include before to enable apartment-aware awaiting. + //! Returns an awaitable that resumes the caller in the same COM apartment that was current when the await began. + //! This matches @ref wil::com_task's default, so it is used mainly on a @ref wil::task to override arbitrary-thread + //! resumption for a single `co_await`. + //! @note You must `#include ` before `` to enable apartment-aware awaiting. + //! @return An awaitable for the task's result that completes in the same COM apartment as the awaiter. auto resume_same_apartment() && noexcept; + /// @cond // Compiler error message metaprogramming: Tell people that they // need to use std::move() if they try to co_await an lvalue. struct cannot_await_lvalue_use_std_move @@ -618,11 +632,21 @@ struct task_base { } }; + /// @endcond + + //! Deleted: a task must be awaited as an rvalue, not an lvalue. + //! Awaiting an lvalue is rejected at compile time; use `co_await wistd::move(t)`. (Awaiting consumes the task, which is why + //! the task types are move-only.) cannot_await_lvalue_use_std_move operator co_await() & = delete; - // You must #include (usually via ) to enable synchronous waiting. + //! Synchronously waits for the task to complete and returns its result. + //! Blocks the calling thread (the usual caveats about synchronously waiting on an STA thread apply) and must be called on an + //! rvalue: `something().get()` or `wistd::move(t).get()`. + //! @note You must `#include ` (usually via ``) to enable synchronous waiting. + //! @return The value produced by the coroutine (or `void`). decltype(auto) get() &&; + /// @cond protected: task_base(task_promise* initial = nullptr) noexcept : promise(initial) { @@ -634,6 +658,7 @@ struct task_base static_cast(*this) = wistd::move(other); return *self; } + /// @endcond private: promise_ptr promise; @@ -641,15 +666,49 @@ struct task_base static void __stdcall wake_by_address(void* completed); }; } // namespace wil::details::coro -/// @endcond namespace wil { // Must write out both classes separately // Cannot use deduction guides with alias template type prior to C++20. +/** An awaitable type, whose `co_await` resumes on an arbitrary thread by default. +A move-only awaitable object that can be awaited at most once; `co_await` takes ownership, so an lvalue must be moved from +(`co_await wistd::move(t)`). + +By default, awaiting a `wil::task` resumes on an arbitrary thread. Override this before the `co_await` with +@ref resume_same_apartment() to resume in the COM apartment that was current when the await began: +`co_await task_function().resume_same_apartment()`. Or change the default by converting to @ref com_task: +`co_await wil::com_task(task_function())`. Wait synchronously with @ref get(): `co_await task_function().get()`. + +The task cannot be cancelled, and an unobserved exception from an abandoned coroutine (its task destroyed without ever being +awaited) is ignored. `wil::task` supplements PPL and C++/WinRT rather than replacing them. +~~~ +// A coroutine that produces a value and (by default) resumes its awaiter on an arbitrary thread. +wil::task GetNameAsync() +{ + co_await resume_background(); + wil::unique_cotaskmem_string name; + THROW_IF_FAILED(GetNameSlow(&name)); + co_return name; +} + +winrt::IAsyncAction UpdateNameAsync() +{ + auto name = co_await GetNameAsync(); // resumes on an arbitrary thread + name = co_await GetNameAsync().resume_same_apartment(); // resumes in the original COM apartment + name = co_await wil::com_task(GetNameAsync()); // convert to a wil::com_task; same as calling resume_same_apartment + + auto task = GetNameAsync(); + // name = co_await task; <--- Compile error: cannot co_await an lvalue + name = co_await wistd::move(task); // OKAY: co_await takes ownership of the task +} +~~~ +@note Synchronous waiting with `get()` requires including `` (usually via ``) before ``. +@tparam T The task's result type; may be `void`, a reference, or any copyable, movable, or move-only object. */ template struct task : details::coro::task_base { + /// @cond using base = details::coro::task_base; // Constructing from task_promise* cannot be explicit because get_return_object relies on implicit conversion. task(details::coro::task_promise* initial = nullptr) noexcept : base(initial) @@ -664,16 +723,51 @@ struct task : details::coro::task_base } using base::operator co_await; + /// @endcond + //! Awaits the task, resuming the caller on an arbitrary thread (shorthand for `resume_any_thread()`). auto operator co_await() && noexcept { return wistd::move(*this).resume_any_thread(); } }; +/** An awaitable type, whose `co_await` resumes in the same COM apartment by default. +A move-only awaitable object that can be awaited at most once; `co_await` takes ownership, so an lvalue must be moved from +(`co_await wistd::move(t)`). + +By default, awaiting a `wil::com_task` resumes in the COM apartment that was current when the await began (particularly convenient +for ASTA/STA work). Override this before the `co_await` with @ref resume_any_thread() to resume on an arbitrary thread: +`co_await com_task_function().resume_any_thread()`. Or change the default by converting to @ref task: +`co_await wil::task(com_task_function())`. Wait synchronously with @ref get(): `co_await com_task_function().get()`. + +The task cannot be cancelled, and an unobserved exception from an abandoned coroutine (its task destroyed without ever being +awaited) is ignored. `wil::com_task` supplements PPL and C++/WinRT rather than replacing them. +~~~ +// A coroutine that produces a value and (by default) resumes its awaiter in the COM apartment that awaited it. +wil::com_task GetGreetingAsync() +{ + co_await resume_background(); + co_return L"Hello!"; // even though work ran off-thread, the awaiter resumes in its original apartment by default +} + +winrt::IAsyncAction UpdateGreetingAsync() +{ + auto greeting = co_await GetGreetingAsync(); // resumes in the original COM apartment + greeting = co_await GetGreetingAsync().resume_any_thread(); // resumes on an arbitrary thread + greeting = co_await wil::task(GetGreetingAsync()); // convert to a wil::task; same as calling resume_any_thread + + auto task = GetGreetingAsync(); + // greeting = co_await task; <--- Compile error: cannot co_await an lvalue + greeting = co_await wistd::move(task); // OKAY: co_await takes ownership of the task +} +~~~ +@note Same-apartment resumption requires including COM headers (e.g. ``) before ``. +@tparam T The task's result type; may be `void`, a reference, or any copyable, movable, or move-only object. */ template struct com_task : details::coro::task_base { + /// @cond using base = details::coro::task_base; // Constructing from task_promise* cannot be explicit because get_return_object relies on implicit conversion. com_task(details::coro::task_promise* initial = nullptr) noexcept : base(initial) @@ -688,7 +782,9 @@ struct com_task : details::coro::task_base } using base::operator co_await; + /// @endcond + //! Awaits the task, resuming the caller in the same COM apartment (shorthand for `resume_same_apartment()`). auto operator co_await() && noexcept { // You must #include before to enable non-agile awaiting. @@ -696,12 +792,15 @@ struct com_task : details::coro::task_base } }; +/// @cond template task(com_task&&) -> task; template com_task(task&&) -> com_task; +/// @endcond } // namespace wil +/// @cond template struct __WI_COROUTINE_NAMESPACE::coroutine_traits, Args...> { @@ -713,6 +812,7 @@ struct __WI_COROUTINE_NAMESPACE::coroutine_traits, Args...> { using promise_type = wil::details::coro::task_promise; }; +/// @endcond #endif // __WIL_COROUTINE_INCLUDED