Skip to content

Fix integral conversion and brace elision warnings - #1621

Open
Daniel Jump (DanielJump) wants to merge 1 commit into
microsoft:masterfrom
DanielJump:warnings-integral-conversions
Open

Daniel Jump (DanielJump) wants to merge 1 commit into
microsoft:masterfrom
DanielJump:warnings-integral-conversions

Conversation

@DanielJump

Copy link
Copy Markdown
Member

These sites produce MSVC warnings under /Wall on both x64 and x86, which forces consumers building at high warning levels to wrap every C++/WinRT include in #pragma warning blocks.

Warning Sites Cause
C5246 most of base_identity.h std::array aggregate init needs two brace levels
C4365 base_collections_base.h, base_collections_vector.h, base_string.h, base_error.h, base_types.h integral promotion, and uint32_t -> ptrdiff_t iterator arithmetic on 32-bit
C4826 base_events.h reinterpret_cast of a pointer to int64_t sign-extends
C4946 base_implements.h reinterpret_cast between related classes; every abi type derives from unknown_abi, so static_cast is correct

Two are worth calling out:

to_hstring used std::copy to widen char to wchar_t. The assignment happens inside <xutility>, so a caller cannot silence it; std::transform with an explicit cast can. to_chars only writes ASCII there, so the cast is exact. Minimal repro:

// cl /std:c++20 /W4 /w14365
#include <algorithm>
#include <iterator>
char narrow[8]{};
wchar_t wide[8]{};
wchar_t* widen() { return std::copy(std::begin(narrow), std::end(narrow), wide); }

is_weak_ref took intptr_t and tested value < 0 while every caller holds a uintptr_t. It now takes uintptr_t and tests the high bit explicitly.

No generated files change, and no public signatures change.

Validated on a translation unit exercising implements<>, delegates, events, all collection flavours, coroutines, agile_ref, weak_ref, boxing and error handling.

Part of #1620.

Consumers that build at high warning levels must currently wrap every
C++/WinRT include in warning pragmas. These sites account for MSVC
C5246 (brace elision), C4365 (signed/unsigned conversion), C4826
(sign-extended pointer cast) and C4946 (reinterpret_cast between
related classes) under /Wall on x64 and x86.

std::array aggregate initialization needs two brace levels. Iterator
arithmetic from uint32_t needs an explicit ptrdiff_t cast on 32-bit.
to_hstring widened char to wchar_t via std::copy, which a caller cannot
silence because the assignment happens inside <xutility>.

The ABI conversions cannot simply use static_cast: a WinRT interface
derives from unknown_abi, but a classic COM interface derives from
::IUnknown and is layout compatible but unrelated. impl::abi_cast picks
static_cast when a conversion exists and falls back to reinterpret_cast
otherwise, so C4946 is avoided wherever the types are actually related.
The same applies in reverse for to_abi, since producer derives from the
interface for classic COM but holds its vtable by value for WinRT.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The warning fixes are behavior-preserving, appropriately scoped, and covered by existing collection, string, event, weak-reference, and COM tests.

0 open findings

What changed in this PR

Eliminates MSVC /Wall warnings in foundational C++/WinRT headers without changing intended behavior or public APIs.

Changes:

  • Adds explicit integral and iterator-offset conversions.
  • Corrects std::array aggregate initialization.
  • Introduces ABI-aware casting and explicit weak-reference/pointer bit handling.
File Description
strings/​base_types.h Makes narrowing conversions explicit.
strings/​base_string.h Fixes reference-count, string-size, and character-widening warnings.
strings/​base_implements.h Adds ABI casting and explicit weak-reference detection.
strings/​base_identity.h Corrects array braces and integral conversions.
strings/​base_events.h Converts encoded pointers without sign-extension warnings.
strings/​base_error.h Explicitly converts HRESULT values for FormatMessageW.
strings/​base_collections_vector.h Uses iterator-compatible offsets.
strings/​base_collections_base.h Uses ptrdiff_t for iterator arithmetic.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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