microsoft / microsoft/cppwinrt

Bug: headers cannot be compiled at high warning levels (MSVC /Wall, clang)

Open
#1,620 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
1.9k
Forks
281
PR merge metrics
No merged PRs in 30d

Description

Version

master (856eaff). Measurements below were taken against the generated projection in windowssdkcppwinrt.10.21320.2, with the strings/ fixes validated at master.

Summary

C++/WinRT headers cannot be compiled at high warning levels. Consumers are forced to wrap every include in #pragma warning blocks. We repackage cppwinrt for the Office monorepo and currently inject a 19-warning push/disable prologue into every generated header as a post-generate step, which is unsatisfying and hides real diagnostics in adjacent code.

I went through that whole suppression list to work out which entries are genuine defects in cppwinrt rather than noise. Most of them are, and all of them are fixable. This issue is to track that; the pull requests are linked at the bottom.

MSVC, measured on the repro below at /Wall:

Code x64 x86 Cause
C5246 114 114 std::array aggregate init needs two brace levels. The guid constructor alone accounts for 57 of these, one per generated guid
C4365 21 31 integral promotion, plus uint32_t to ptrdiff_t iterator arithmetic that only shows up on 32-bit
C4946 16 16 reinterpret_cast between related classes. Every abi type derives from unknown_abi, so static_cast is correct
C4265 15 15 virtual functions with an accessible non-virtual destructor
C4826 0 1 reinterpret_cast of a pointer to int64_t sign-extends

Six further codes we suppress (C4101, C4189, C4456, C4457, C4459, C5205) never fired in any configuration I tried, and C4458 is already suppressed internally by base_delegate.h. C6101 fires under /analyze but all instances are in the Windows SDK's WindowsNumerics.inl, not in cppwinrt.

clang (clang-cl 22.1.2), counting only the warnings we suppress:

Warning Count Notes
-Wnon-virtual-dtor 184 see #1499
-Wunused-variable 60 already fixed at master by [[maybe_unused]]; only affects older packages
-Wnontrivial-memcall 19 already fixed at master by making the base_array.h guard clang-aware
-Wmissing-braces 12 all in the SDK's DirectXMathMatrix.inl, not cppwinrt
-Wshadow 8 all in the MSVC STL, not cppwinrt

So of 19 suppressed warnings, six are real defects in cppwinrt today, two are already fixed at master, and the rest are either stale or owned by the SDK and the STL.

Reproducible example
// cl /c /EHsc /std:c++20 /permissive- /await:strict /Wall repro.cpp
//
// Add /wd for the /Wall codes that are not about cppwinrt:
//   4514 4710 4711 4820 5045 4625 4626 5026 5027 4623 4571 4668 5039 4643 5262 4868 5204
//
// clang-cl /c /EHsc /std:c++20 -Wno-everything -Wnon-virtual-dtor repro.cpp

#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>

using namespace winrt;
using namespace Windows::Foundation;

struct Widget : implements<Widget, IStringable>
{
    hstring ToString() { return L"Widget"; }
};

IAsyncAction Run()
{
    auto widget = make<Widget>();
    auto vector = single_threaded_vector<hstring>({ widget.ToString() });
    auto map = single_threaded_map<hstring, int32_t>();
    map.Insert(vector.GetAt(0), 1);
    co_return;
}

A wider translation unit that also exercises delegates, events, agile_ref, weak_ref, boxing and error handling reaches the counts in the table above.

Separately, to_hstring widens char to wchar_t through std::copy, which produces a C4365 that a caller cannot suppress, because the assignment happens inside <xutility>:

// 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); }
Expected behavior

C++/WinRT headers compile without warnings at MSVC /Wall and under clang, so that consumers do not have to wrap includes in pragmas or globally relax their own analysis rules.

Actual behavior

The example above produces 96 warnings at /Wall on x64 (C5246 x67, C4365 x21, C4946 x7, C4265 x1) and 81 -Wnon-virtual-dtor under clang-cl.

The wider translation unit reaches 166 warnings on x64, 177 on x86, and 304 under clang.

With #1621, #1622 and #1623 applied it is 0 on x64, 0 on x86, and 0 attributable to cppwinrt under clang.

Additional comments

Related, both closed without a fix:

  • #1499 reports -Wnon-virtual-dtor and suggests virtual ~type() noexcept = default;. That fix would be incorrect for an ABI type, since adding a virtual destructor adds a vtable slot and breaks binary compatibility (confirmed with /d1reportSingleClassLayout). #1623 suppresses the warning inside the generated headers instead.
  • #1585 is a consumer hitting C4265, C4946 and C5204 and being forced to disable code analysis rules globally, ending in a CAExcludePath workaround.
Pull requests

Split so the risk is separable. All three are green on CI.

PR Change Warnings
#1621 Integral conversions and brace elision. Mechanical casts and brace pairs, plus an impl::abi_cast helper so the ABI conversions are correct for classic COM interfaces as well as WinRT ones. No signature or generated-file changes. C4365, C4946, C4826, and the non-guid C5246
#1622 Adds a guid constructor overload taking std::uint8_t const (&)[8] alongside the existing std::array<std::uint8_t, 8> const& one. Additive, so nothing is source-breaking. C5246, 114 -> 20 on its own; the remainder is covered by #1621
#1623 Scoped #pragma suppression of the destructor warnings in each generated header, after review feedback that changing the vtable layout of ABI types was not acceptable. C4265, -Wnon-virtual-dtor

While measuring the above I also found that base_macros.h disables four warnings with no matching push / pop, so they leak into every translation unit that includes <winrt/base.h> and silently disable those warnings in the consumer's own code. That is an independent bug, filed as #1624 and fixed in #1625.

Happy to reshape or drop any of these if you would rather solve it differently.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by running the repro under MSVC /Wall and clang-cl, then inspect the generated headers and base_macros.h. Read PRs #1621, #1622, and #1623 to understand the changes already under way. Done means the repro produces no warnings attributable to cppwinrt without consumer warning pragmas.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.