1445013 - Consider adding a lint against including <windows.h> in headers

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

<windows.h> defines a ton of random macros and other stuff which can really mess with compiling code in part of Gecko. This means that many files need to avoid including the header. Unfortunately, right now we have a bunch of headers in-tree which include <windows.h> in them, so it's very easy to accidentally end up in a situation where <windows.h> is included in your file, and then you get to go on a fun witch hunt in order to discover why.

If we can get to the point where no headers (or a small set of whitelisted ones) include <windows.h>, we can avoid having this problem crop up again in the future by adding a lint against it.

This could be implemented as a clang-plugin pass, but it might be just as efficient to simply check the status of `git grep '<windows.h>' -- *.h` (which has the advantage of working on all platforms), and failing the build if anything is found.

If that is too aggressive, we could also modify mozbuild to search header files marked EXPORT for <windows.h> - so that we don't prevent internal headers from importing that file. While we roll out a check like this, we could have a flag which allows you to export <windows.h> which you can set in your moz.build file.

Comment 1

[Chris Peterson [:cpeterson]]

This lint would be very easy to add using mozlint yml file, which would be run on Treeherder check-ins. It can also exclude files or directories for third-party code or Mozilla code that hasn't been fixed yet.

https://firefox-source-docs.mozilla.org/tools/lint/create.html

Here is a similar mozlint checking the usage of virtual/override/final function specifiers:

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a00e515def3

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

I think the hard part here is removing windows.h from all the places we currently have it.  For example, ipdl generated code pulls it in.  I don't think we should lock this down until we get this kind of basic infrastructure clear of the window header.  Otherwise we just make its very hard for people to do their job if they need to write an IPC actor.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Closing as wontfix due to bug 1448426 making windows.h in headers much less damaging.