[clang] [llvm] [Clang] Match MSVC handling of duplicate header search paths in Microsoft compatibility modes. (PR #105738)

[Tom Honermann via cfe-commits]

tahonermann wrote:

There is some interesting MSVC behavior that I'm not planning to replicate with the changes being done for this issue but that I thought are worth documenting (here at least, perhaps in Clang docs as well).

MSVC documentation for the [`/external`](https://learn.microsoft.com/en-us/cpp/build/reference/external-external-headers-diagnostics?view=msvc-170) suite of options includes the `/external:Wn` option (where `Wn` is one of `W0`, `W1`, `W2`,`W3`, or `W4`) to specify the warning level that is used for "external" header search paths; paths specified with the `/external:I` or `/external:env` option, the `%EXTERNAL_INCLUDE%` environment variable, or included with a angle bracket enclosed path (`#include <file.h>`) when the `/external:anglebrackets` option is enabled. The warning levels are meaningless to Clang, so Clang currently maps `/external:W0` to `-Wno-system-headers` and the rest to `-Wsystem-headers`. Clang does not yet implement `/external:anglebrackets`. This all seems fine.

There are two interesting behaviors that MSVC exhibits related to the `/I` and `%INCLUDE%` environment variable.

1. By default, paths specified by `/I` or `%INCLUDE%` are treated as user paths; whether warnings are issued for them is subject to use of one of the [warning level options](https://learn.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=msvc-170) like `/Wall` and `/W4`; the `/external:Wn` option has no effect on them.
2. However, when a path specified by `/external:I`, `/external:env`, or `%EXTERNAL_INCLUDE%` matches the *beginning* of a path in a `/I` or `%INCLUDE%` path, then the `/external:Wn` specified warning level is applied to that path. The path match does not have to occur at a path component boundary (unless the external path ends in a path separator). Matched paths are treated like system paths.

In the following example, each header file has code that triggers [MSVC warning C4245](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4245?view=msvc-170); a warning that is issued at warning level 4.

```
> type incbar\a.h
const unsigned int sysinc1 = -1;

> type incbaz\b.h
const unsigned int sysinc2 = -1;

> type incfoo\c.h
const unsigned int extinc = -1;

> type t.cpp
#include <a.h>
#include <b.h>
#include <c.h>

> set INCLUDE=incbar;incbaz;incfoo

# No warnings issued by default.
> cl /nologo /c t.cpp
t.cpp

# Warnings issued with `/W4`.
> cl /nologo /c /W4 t.cpp
t.cpp
incbar\a.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
incbaz\b.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
incfoo\c.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch

# No warnings issued with `/external:W4`.
> cl /nologo /c /external:W4 t.cpp
t.cpp

# Warnings issued for all three search directories with `/external:W4` and `/external:I inc`.
# Not shown here, but if a `inc` directory existed, it would also be searched for header files.
> cl /nologo /c /external:W4 /external:I inc t.cpp
t.cpp
incbar\a.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
incbaz\b.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
incfoo\c.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch

# Warnings issued for only `incbar/a.h` and `incbaz.h` with `/external:W4` and `/external:I incb`.
# Not shown here, but if a `incb` directory existed, it would also be searched for header files.
> cl /nologo /c /external:W4 /external:I incb t.cpp
t.cpp
incbar\a.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
incbaz\b.h(1): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch

# No warnings issued when the external include path ends with a path separator that doesn't match another path.
# Not shown here, but if a `incb` directory existed, it would be searched for header files.
> cl /nologo /c /external:W4 /external:I incb\ t.cpp
t.cpp
```

My intent with changes made for this issue is to (continue to) treat all paths specified by `/I` as user paths and all paths specified by `/external:I`, `/external:env`, `%INCLUDE%`, or `%EXTERNAL_INCLUDE%` as system paths.

Ideally, I think we would do the following at some point to improve compatibility with MSVC.

1. Treat paths specified by `%INCLUDE%` as user paths by default.
2. After building the header search path in `InitHeaderSearch::Realize()`, alter the `SrcMgr::CharacteristicKind` value stored in the `Lookup` member of `DirectoryLookupInfo` in the `IncludePath` member of `InitHeaderSearch` to indicate a system path for each path that is a prefix match for an external path (where the prefix need not match on a path component boundary).

https://github.com/llvm/llvm-project/pull/105738