What's the best way to deal with warnings in Windows system includes?

[Justin TerAvest]

I'm trying to enable warnings for C4668 in native_client builds to force errors
for the case where necessary macro values aren't defined.


However, adding "/we4668" to the MSVS build triggers a bunch of errors from
system includes, like the following:

d:\src\depot_tools\win_toolchain\vs2013_files\win8sdk\include\um\processthreadsapi.h(1170)
: error C4668: '_WIN32_WINNT_WINTHRESHOLD' is not defined as a preprocessor
macro, replacing with '0' for '#if/#elif'
d:\src\depot_tools\win_toolchain\vs2013_files\win8sdk\include\um\winbase.h(8618)
: error C4668: 'NTDDI_WIN7SP1' is not defined as a preprocessor macro,
replacing with '0' for '#if/#elif'

The advice I've found is to wrap the problematic includes with
pragmas, like this:
#pragma warning(push)
#pragma warning(disable:4668)
//Some includes with unfixable warnings
#pragma warning(pop)

Presumably we've run into this before when new warnings have been
enabled on Windows.
Is there a better way to deal with warnings coming from "system" includes?

Thanks!

[Peter Kasting]

(1) Make sure you file bugs for this upstream with the Visual Studio team, so they can fix them.

(2) In the interim, you need to either:

  (a) do the wrapping you describe above,

  (b) create your own wrapper header that simply wraps the system header as above, then include that wrapper header instead, or

  (c) disable particular warnings for specific projects that have source files including these files.

PK

[Scott Graham]

On Wed, Dec 17, 2014 at 1:42 PM, Justin TerAvest <tera...@chromium.org> wrote:

Not that I know of, unfortunately. We generally just don't enable them if the system headers aren't clean.

It's probably limited to a few headers? So including a something/win.h instead of windows.h, winsock2.h, etc. directly might be enough to hide the pragma goop, at least for non-interface headers.

 

Thanks!

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
    http://groups.google.com/a/chromium.org/group/chromium-dev

[Nico Weber]

On Wed, Dec 17, 2014 at 1:46 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Wed, Dec 17, 2014 at 1:42 PM, Justin TerAvest <tera...@chromium.org> wrote:

I'm trying to enable warnings for C4668 in native_client builds to force errors
for the case where necessary macro values aren't defined.


However, adding "/we4668" to the MSVS build triggers a bunch of errors from
system includes, like the following:

d:\src\depot_tools\win_toolchain\vs2013_files\win8sdk\include\um\processthreadsapi.h(1170)
: error C4668: '_WIN32_WINNT_WINTHRESHOLD' is not defined as a preprocessor
macro, replacing with '0' for '#if/#elif'
d:\src\depot_tools\win_toolchain\vs2013_files\win8sdk\include\um\winbase.h(8618)
: error C4668: 'NTDDI_WIN7SP1' is not defined as a preprocessor macro,
replacing with '0' for '#if/#elif'

The advice I've found is to wrap the problematic includes with
pragmas, like this:
#pragma warning(push)
#pragma warning(disable:4668)
//Some includes with unfixable warnings
#pragma warning(pop)

Presumably we've run into this before when new warnings have been
enabled on Windows.
Is there a better way to deal with warnings coming from "system" includes?

(1) Make sure you file bugs for this upstream with the Visual Studio team, so they can fix them.

 

For what it's worth, other compilers don't emit warnings in system headers by default (on Windows, as determined by the INCLUDE env var). If nobody has done this yet, maybe also file an upstream bug for cl to learn this trick – this would solve all problems with warnings in system headers as long as we set INCLUDE correctly.

[Yuriy Solodkyy]

This is an old thread, but just FYI, the Visual C++ team have just added support for warning levels in external headers in VS 2017 version 15.6. You can find the details in their blog post: Broken Warnings Theory.

In essence, it it is a moral equivalent of -isystem that does automatically what many developers were doing manually: pushes new warning level right before #include directive and pops it up right after. There are additional flags to specify locations of external headers, flag to treat all <> includes as external, #pragma system_header and a feature not available in Clang or GCC (as of this writing) to see warnings in external headers across template instantiation stack when the template was instantiated in the user code.

Besides the comments under their blog post, you can also find some useful discussion in a reddit announcement for that post.

Yuriy

[Nico Weber]

Thanks for the note, Yuri! That looks pretty comprehensive.

Does cl.exe treat any headers as external headers by default? For example, clang-cl treats everything on %INCLUDE% as system header by default (since that's where users usually get Windows.h etc from). Would every project have to explicitly say /external:env:INCLUDE or is there something that makes sure that SDK headers are marked as external headers by default already?

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/f3c354ea-8739-46d2-a355-32800e357bd2%40chromium.org.

[Yuriy Solodkyy]

Hi Nico,

We've considered it and in previous internal builds we had path in INCLUDE variable implicitly assumed to be external. We've removed that behavior for the current official release for several reasons: 

• we didn't want to have any unexpected behavior changes for users without explicit opt in. Granted, without /external:W0 this implicit change wouldn't change any behavior, but it might still expose the two points below.
• in case of potential bugs, we didn't want those exposed onto everyone
  
• we also check external include directories if they are present and give warning if they aren't (like Clang and GCC does). 
  

The latter one was added close to the deadline to see if people like such warning and whether we should consider it for regular includes, but we didn't want it to have any untested performance implications, which implicit /external:env:INCLUDE could have exposed. That said, while this is still an experimental feature we are open to any suggestions you guys might have that would help your workflow. Making something default behavior would probably require an overwhelming support, which we hope to see from switch usage of those users who opted into sharing usage stats with us.

Cheers,

Yuriy