microsoft / microsoft/cppwinrt
base_macros.h disables warnings without push/pop, leaking them into consumer translation units
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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