microsoft / microsoft/cppwinrt

base_macros.h disables warnings without push/pop, leaking them into consumer translation units

Open
#1,624 0 comments 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

Description

strings/base_macros.h disables four MSVC warnings with bare #pragma warning(disable: ...) and no matching push / pop:

#ifdef _MSC_VER
// Note: this is a workaround for a false-positive warning produced by the Visual C++ 15.9 compiler.
#pragma warning(disable : 5046)

// Note: this is a workaround for a false-positive warning produced by the Visual C++ 16.3 compiler.
#pragma warning(disable : 4268)

// C++ module warnings by /W4
#pragma warning(disable : 4499)
#pragma warning(disable : 4630)
#endif // _MSC_VER

base.h includes base_macros.h near the top and never restores the warning state. C5046, C4268, C4499 and C4630 therefore stay disabled for the remainder of every translation unit that includes <winrt/base.h>, which includes all of the consumer's own code that follows the include.

The first two disables are documented as workarounds for Visual C++ 15.9 and 16.3, while the latter two were added for early C++ module support. Current supported toolsets no longer emit these warnings for C++/WinRT, so retaining the pragmas only suppresses diagnostics in consumer code.

Repro
// leak.cpp
namespace { struct S { int x; }; }
S f();
int main() { f(); return 0; }
cl /c /std:c++20 /permissive- /W4 leak.cpp
leak.cpp(3): warning C5046: 'f': Symbol involving type with internal linkage not defined

Add the include and the warning disappears, even though the offending code is unchanged and is entirely outside C++/WinRT:

#include <winrt/base.h>
namespace { struct S { int x; }; }
S f();
int main() { f(); return 0; }
cl /c /std:c++20 /permissive- /W4 /I<sdk> leak.cpp
(no diagnostics)

Reproduced with both /W4 and /Wall on MSVC x64.

This is the only unscoped suppression in the tree

Auditing every warning pragma under strings/:

header MSVC push / pop / disable clang push / pop / ignored
base_activation.h 1 / 1 / 1 1 / 1 / 1
base_composable.h 1 / 1 / 1 -
base_delegate.h 1 / 1 / 1 -
base_error.h - 1 / 1 / 1
base_fast_forward.h - 1 / 1 / 1
base_implements.h 1 / 1 / 1 1 / 1 / 1
base_macros.h 0 / 0 / 4 -

Every other suppression in the project is already correctly scoped, so this looks like an oversight rather than a deliberate choice.

Fix

Remove all four obsolete disables from base_macros.h. This fixes every inclusion path uniformly and avoids adding warning-state plumbing for diagnostics that current compilers no longer produce.

The official MSVC off-by-default warning list does not include any of the four warning numbers. A rich generated-header translation unit compiles under /Wall with both v143 and v145 without any of them, and the v145 named-module target also rebuilds under /Wall without any of them. The original consumer repro reports C5046 again.

Fixed in #1625.

Relationship to #1623

#1623 adds a file-level diagnostic scope to every generated header, which would incidentally contain these disables inside base.h as a side effect. I am raising and fixing this separately because it is a distinct bug with its own repro, and because it should be fixed whether or not #1623 is taken.

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

Inspect strings/base_macros.h and the MSVC warning pragmas described in the issue. Reproduce the leak with the provided leak.cpp example under /W4 or /Wall, then verify that the warnings return after the change and that the relevant C++/WinRT headers still build cleanly; the issue notes this was fixed in #1625.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
tooling
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.