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

2025-01-19 Thread Tom Honermann via cfe-commits

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

>From c5d4a8b94d0f5b294dda30b455c492e0a860906a Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Support for MSVC compatible header search path
 ordering.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the MSVC `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

Paths specified by the MSVC `/external:I` or `/external:env` options or by the
`EXTERNAL_INCLUDE` environment variable are not just used to nominate a header
search path. MSVC also uses these paths as external directory prefixes to mark
paths that are to be treated as system directories. When a header file is
resolved to a path for which one of these paths is a prefix match, that header
file is treated as a system header regardless of whether the header file was
resolved against a `/I` specified path. Note that it is not necessary for the
final path component of the external path to match a directory in the filesystem
in order to be used as an external directory prefix, though trailing path
separators are significant. For example:

   Include directive Command line System header
     --   -
   #include   /I. /external:Ifoo   Yes
   #include   /I. /external:Ifoo\  No

This change adds support for the MSVC external path concept through the addition
of new driver options with the following option syntax.
   clang clang-cl
     ---
   -iexternal   /external:I 
   -iexternal-env=  /external:env:

Paths specified by these options are treated as system paths. That is, whether
warnings are issued in header files found via these paths remains subject to
use of the `-Wsystem-headers` and `-Wno-system-headers` options. Note that the
MSVC `/external:W` options are mapped to these options.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior, and Clang's
historical behavior, of suppressing user paths that match a system path
regardless of how each is specified. In order to support both behaviors, the
following driver option has been added. The option arguments shown reflect the
default behavior for each driver.

   clang clang-cl
     -
   -fheader-search=gcc   -fheader-search=microsoft

Use of the MSVC compatible header search path order by default for `clang-cl`
is a change in behavior with potential to cause p

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

2025-01-17 Thread Tom Honermann via cfe-commits

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

>From 1c9813eef03380f0013eaedf669612566a97bce1 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Support for MSVC compatible header search path
 ordering.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the MSVC `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

Paths specified by the MSVC `/external:I` or `/external:env` options or by the
`EXTERNAL_INCLUDE` environment variable are not just used to nominate a header
search path. MSVC also uses these paths as external directory prefixes to mark
paths that are to be treated as system directories. When a header file is
resolved to a path for which one of these paths is a prefix match, that header
file is treated as a system header regardless of whether the header file was
resolved against a `/I` specified path. Note that it is not necessary for the
final path component of the external path to match a directory in the filesystem
in order to be used as an external directory prefix, though trailing path
separators are significant. For example:

   Include directive Command line System header
     --   -
   #include   /I. /external:Ifoo   Yes
   #include   /I. /external:Ifoo\  No

This change adds support for the MSVC external path concept through the addition
of new driver options with the following option syntax.
   clang clang-cl
     ---
   -iexternal   /external:I 
   -iexternal-env=  /external:env:

Paths specified by these options are treated as system paths. That is, whether
warnings are issued in header files found via these paths remains subject to
use of the `-Wsystem-headers` and `-Wno-system-headers` options. Note that the
MSVC `/external:W` options are mapped to these options.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior, and Clang's
historical behavior, of suppressing user paths that match a system path
regardless of how each is specified. In order to support both behaviors, the
following driver option has been added. The option arguments shown reflect the
default behavior for each driver.

   clang clang-cl
     -
   -fheader-search=gcc   -fheader-search=microsoft

Use of the MSVC compatible header search path order by default for `clang-cl`
is a change in behavior with potential to cause p

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

2025-01-16 Thread Tom Honermann via cfe-commits

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

>From 21b213c319748567f469a64cb3627215d5787352 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Support for MSVC compatible header search path
 ordering.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the MSVC `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

Paths specified by the MSVC `/external:I` or `/external:env` options or by the
`EXTERNAL_INCLUDE` environment variable are not just used to nominate a header
search path. MSVC also uses these paths as external directory prefixes to mark
paths that are to be treated as system directories. When a header file is
resolved to a path for which one of these paths is a prefix match, that header
file is treated as a system header regardless of whether the header file was
resolved against a `/I` specified path. Note that it is not necessary for the
final path component of the external path to match a directory in the filesystem
in order to be used as an external directory prefix, though trailing path
separators are significant. For example:

   Include directive Command line System header
     --   -
   #include   /I. /external:Ifoo   Yes
   #include   /I. /external:Ifoo\  No

This change adds support for the MSVC external path concept through the addition
of new driver options with the following option syntax.
   clang clang-cl
     ---
   -iexternal   /external:I 
   -iexternal-env=  /external:env:

Paths specified by these options are treated as system paths. That is, whether
warnings are issued in header files found via these paths remains subject to
use of the `-Wsystem-headers` and `-Wno-system-headers` options. Note that the
MSVC `/external:W` options are mapped to these options.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior, and Clang's
historical behavior, of suppressing user paths that match a system path
regardless of how each is specified. In order to support both behaviors, the
following driver option has been added. The option arguments shown reflect the
default behavior for each driver.

   clang clang-cl
     -
   -fheader-search=gcc   -fheader-search=microsoft

Use of the MSVC compatible header search path order by default for `clang-cl`
is a change in behavior with potential to cause p

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

2025-01-16 Thread Tom Honermann via cfe-commits

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

>From 9dc8727fad50d7a808fc14173189105675cb46c9 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Support for MSVC compatible header search path
 ordering.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the MSVC `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

Paths specified by the MSVC `/external:I` or `/external:env` options or by the
`EXTERNAL_INCLUDE` environment variable are not just used to nominate a header
search path. MSVC also uses these paths as external directory prefixes to mark
paths that are to be treated as system directories. When a header file is
resolved to a path for which one of these paths is a prefix match, that header
file is treated as a system header regardless of whether the header file was
resolved against a `/I` specified path. Note that it is not necessary for the
final path component of the external path to match a directory in the filesystem
in order to be used as an external directory prefix, though trailing path
separators are significant. For example:

   Include directive Command line System header
     --   -
   #include   /I. /external:Ifoo   Yes
   #include   /I. /external:Ifoo\  No

This change adds support for the MSVC external path concept through the addition
of new driver options with the following option syntax.
   clang clang-cl
     ---
   -iexternal   /external:I 
   -iexternal-env=  /external:env:

Paths specified by these options are treated as system paths. That is, whether
warnings are issued in header files found via these paths remains subject to
use of the `-Wsystem-headers` and `-Wno-system-headers` options. Note that the
MSVC `/external:W` options are mapped to these options.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior, and Clang's
historical behavior, of suppressing user paths that match a system path
regardless of how each is specified. In order to support both behaviors, the
following driver option has been added. The option arguments shown reflect the
default behavior for each driver.

   clang clang-cl
     -
   -fheader-search=gcc   -fheader-search=microsoft

Use of the MSVC compatible header search path order by default for `clang-cl`
is a change in behavior with potential to cause p

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

2025-01-06 Thread Tom Honermann via cfe-commits

tahonermann wrote:

I've been continuing to work on this, but have yet to get all of our internal 
tests to pass. I've reverted the PR to a draft pending successful internal 
testing.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2025-01-06 Thread Tom Honermann via cfe-commits

https://github.com/tahonermann converted_to_draft 
https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-12-10 Thread Tom Honermann via cfe-commits


@@ -3190,19 +3190,27 @@ static void GenerateHeaderSearchArgs(const 
HeaderSearchOptions &Opts,
   auto It = Opts.UserEntries.begin();
   auto End = Opts.UserEntries.end();
 
-  // Add -I... and -F... options in order.
-  for (; It < End && Matches(*It, {frontend::Angled}, std::nullopt, true);
+  // Add the -I..., -F..., and -iexternal options in order.
+  for (; It < End && Matches(*It, {frontend::Angled, frontend::External},
+ std::nullopt, true);
++It) {
 OptSpecifier Opt = [It, Matches]() {
   if (Matches(*It, frontend::Angled, true, true))
 return OPT_F;
   if (Matches(*It, frontend::Angled, false, true))
 return OPT_I;
+  if (Matches(*It, frontend::External, false, true))

tahonermann wrote:

In this case, the call is to a local lambda and there are a lot of such calls. 
Adding the parameter names pushes the line length therefore requiring line 
wrapping. I don't think it is an improvement in this case, but if you insist, 
I'll make the change.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-12-10 Thread Tom Honermann via cfe-commits

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

>From 8ecdbe842c2e42be9247788ed16d6021e9488f19 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/2] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  15 +-
 clang/lib/Driver/ToolChai

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

2024-12-10 Thread Tom Honermann via cfe-commits

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

>From d5c7035fd0c007e9833150a3d1b0a86d8744aa5d Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/2] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  15 +-
 clang/lib/Driver/ToolChai

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

2024-12-06 Thread Shafik Yaghmour via cfe-commits


@@ -3190,19 +3190,27 @@ static void GenerateHeaderSearchArgs(const 
HeaderSearchOptions &Opts,
   auto It = Opts.UserEntries.begin();
   auto End = Opts.UserEntries.end();
 
-  // Add -I... and -F... options in order.
-  for (; It < End && Matches(*It, {frontend::Angled}, std::nullopt, true);
+  // Add the -I..., -F..., and -iexternal options in order.
+  for (; It < End && Matches(*It, {frontend::Angled, frontend::External},
+ std::nullopt, true);
++It) {
 OptSpecifier Opt = [It, Matches]() {
   if (Matches(*It, frontend::Angled, true, true))
 return OPT_F;
   if (Matches(*It, frontend::Angled, false, true))
 return OPT_I;
+  if (Matches(*It, frontend::External, false, true))

shafik wrote:

We should be striving to use 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)
 for literals like this. It looks like we do so below, it is unfortunate it is 
not applied consistently.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-12-06 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

nitpick

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-12-06 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited 
https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-12 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> Opinions on whether the current `-fms-compatibility` controlled behavior to 
> search the directories of the include stack for `#include "xxx"` headers 
> should be switched to this new option is welcome. I tend to think it should 
> be.

My intuition is that it should be switched to the new option; we want 
`-fms-compatibility` to mean "be compatible with MSVC" and that shouldn't 
require additional flags except in exceptional cases. This doesn't feel like an 
exceptional case to me.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-08 Thread Tom Honermann via cfe-commits

tahonermann wrote:

Further internal testing has indicated that there will likely be backward 
compatibility problems in practice with the implemented approach of 
differentiating behavior based on `-fms-compatibility`. I'm therefore going to 
look into adding a new `-cc1` option, `-fms-header-search`, to control this 
behavior. The new option will only be enabled by default for `clang-cl`.

Opinions on whether the current `-fms-compatibility` controlled behavior to 
search the directories of the include stack for `#include "xxx"` headers should 
be switched to this new option is welcome. I tend to think it should be.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-08 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> > Doesn't that impact whether a header is found via `""` or `<>` syntax? 
> > e.g., if something went from a user header to a system header, I thought 
> > that meant the search order would then change depending on which include 
> > syntax you used?
> 
> No, those are orthogonal concerns, but there is unfortunate terminology 
> overlap. Options like `-isystem` nominate system include paths and headers 
> found within those paths are treated as system headers. However, a header 
> found via another include path can still be considered a system header. 
> Consider a header file that contains `#pragma GCC system_header`; such a 
> header file is considered a system header, but doesn't influence how header 
> file inclusion is resolved.

Okay, thank you! Then this seem good to me.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-07 Thread Tom Honermann via cfe-commits

tahonermann wrote:

> Doesn't that impact whether a header is found via `""` or `<>` syntax? e.g., 
> if something went from a user header to a system header, I thought that meant 
> the search order would then change depending on which include syntax you used?

No, those are orthogonal concerns, but there is unfortunate terminology 
overlap. Options like `-isystem` nominate system include paths and headers 
found within those paths are treated as system headers. However, a header found 
via another include path can still be considered a system header. Consider a 
header file that contains `#pragma GCC system_header`; such a header file is 
considered a system header, but doesn't influence how header file inclusion is 
resolved.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-07 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> Thanks, @AaronBallman,
> 
> > > Ideally, I think we would do the following at some point to improve 
> > > compatibility with MSVC.
> > 
> > 
> > I'm not opposed, but I am concerned about the potential to subtly break 
> > user code that's relying on our current search path behavior. We may need 
> > to find some clever diagnostics for cases where lookup would have 
> > previously succeeded or found a different file.
> 
> The changes I was suggesting in that comment would not change what files are 
> found; it would only affect whether the file was to be treated as a user or 
> system header. 

Doesn't that impact whether a header is found via `""` or `<>` syntax? e.g., if 
something went from a user header to a system header, I thought that meant the 
search order would then change depending on which include syntax you used?

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-06 Thread Tom Honermann via cfe-commits

tahonermann wrote:

Friendly ping for additional code review. @rnk, @nico, @zmodem, @majnemer, 
@zygoloid.

The Windows build is failing spuriously. I've rebased and force pushed multiple 
times to try to get a clean build, but it keeps failing during git checkout.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-06 Thread Tom Honermann via cfe-commits

tahonermann wrote:

Thanks, @AaronBallman,

> > Ideally, I think we would do the following at some point to improve 
> > compatibility with MSVC.
> 
> I'm not opposed, but I am concerned about the potential to subtly break user 
> code that's relying on our current search path behavior. We may need to find 
> some clever diagnostics for cases where lookup would have previously 
> succeeded or found a different file.

The changes I was suggesting in that comment would not change what files are 
found; it would only affect whether the file was to be treated as a user or 
system header. That being said, the changes in this PR do affect file 
resolution, but only in cases where the same path is specified multiple times.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-06 Thread Tom Honermann via cfe-commits

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

>From d5c7035fd0c007e9833150a3d1b0a86d8744aa5d Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Match MSVC handling of duplicate header search paths
 in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  15 +-
 clang/lib/Driver/ToolChains/M

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

2024-11-06 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> 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.

I think this option is the least disruptive.

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

I'm not opposed, but I am concerned about the potential to subtly break user 
code that's relying on our current search path behavior. We may need to find 
some clever diagnostics for cases where lookup would have previously succeeded 
or found a different file.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-11-05 Thread Tom Honermann via cfe-commits

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

>From 35e3cb1c9901c365c0cd6225a61067987d6c40b8 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Match MSVC handling of duplicate header search paths
 in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  15 +-
 clang/lib/Driver/ToolChains/M

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

2024-11-04 Thread Tom Honermann via cfe-commits

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

>From a9b1711372a5f1a2294ba5f8b437a6020023a810 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Match MSVC handling of duplicate header search paths
 in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  15 +-
 clang/lib/Driver/ToolChains/M

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

2024-11-04 Thread Tom Honermann via cfe-commits

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

>From 14538f86fba30eec50e3a5a85c4e569af8435048 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Match MSVC handling of duplicate header search paths
 in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  12 +-
 clang/lib/Driver/ToolChains/M

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

2024-10-30 Thread Tom Honermann via cfe-commits

https://github.com/tahonermann edited 
https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-30 Thread Tom Honermann via cfe-commits

tahonermann wrote:

This PR is finally ready for review. I would especially appreciate it if @rnk 
and @nico took a close look. Please add other known stakeholders.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-30 Thread Tom Honermann via cfe-commits

https://github.com/tahonermann ready_for_review 
https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-30 Thread Tom Honermann via cfe-commits

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

>From 346c9693c7c02c82358208f8cf2a36ccab5cb70d Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH] [Clang] Match MSVC handling of duplicate header search paths
 in Microsoft compatibility modes.

Clang has historically matched GCC's behavior for header search path order
and pruning of duplicate paths. That traditional behavior is to order user
search paths before system search paths, to ignore user search paths that
duplicate a (later) system search path, and to ignore search paths that
duplicate an earlier search path of the same user/system kind. This differs
from MSVC and can result in inconsistent header file resolution for `#include`
directives.

MSVC orders header search paths as follows:
1) Paths specified by the `/I` and `/external:I` options are processed in
   the order that they appear. Paths specified by `/I` that duplicate a path
   specified by `/external:I` are ignored regardless of the order of the
   options. Paths specified by `/I` that duplicate a path from a prior `/I`
   option are ignored. Paths specified by `/external:I` that duplicate a
   path from a later `/external:I` option are ignored.
2) Paths specified by `/external:env` are processed in the order that they
   appear. Paths that duplicate a path from a `/I` or `/external:I` option
   are ignored regardless of the order of the options. Paths that duplicate a
   path from a prior `/external:env` option or an earlier path from the same
   `/external:env` option are ignored.
3) Paths specified by the `INCLUDE` environment variable are processed in
   the order they appear. Paths that duplicate a path from a `/I`,
   `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate an earlier path in the `INCLUDE` environment variable are
   ignored.
4) Paths specified by the `EXTERNAL_INCLUDE` environment variable are
   processed in the order they appear. Paths that duplicate a path from a
   `/I`, `/external:I`, or `/external:env` option are ignored. Paths that
   duplicate a path from the `INCLUDE` environment variable are ignored.
   Paths that duplicate an earlier path in the `EXTERNAL_INCLUDE
   environment variable are ignored.

Prior to this change, Clang handled the `/external:I` and `/external:env`
options and the paths present in the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variables as though they were specified with the `-isystem` option.
The GCC behavior described above then lead to a command line such as
`/external:I dir1 /Idir2` having a header search order of `dir2` followed by
`dir1`; contrary to MSVC behavior.

This change adds support for the MSVC external path concept for both the `clang`
and `clang-cl` drivers with the following option syntax. These options match the
MSVC behavior described above for both drivers.
 clangclang-cl
  ---
 -iexternal  /external:I 
 -iexternal-env= /external:env:
Paths specified by these options are still treated as system paths. That is,
whether warnings are issued in header files found via these paths remains
subject to use of the `-Wsystem-headers` and `-Wno-system-headers` options.
In the future, it would make sense to add a separate option that matches the
MSVC `/external:Wn` option to control such warnings.

The MSVC behavior described above implies that (system) paths present in the
`INCLUDE` and `EXTERNAL_INCLUDE` environment variables do not suppress matching
user paths specified via `/I`. This contrasts with GCC's behavior of suppressing
user paths that match a system path regardless of how each is specified. Since
the `clang-cl` driver maps paths from the `INCLUDE` and `EXTERNAL_INCLUDE`
environment variable to `-internal-isystem`, matching MSVC behavior requires
suppressing that aspect of the GCC behavior. With this change, system paths
will no longer suppress user paths when the `-fms-compatibility` option is
explicitly or implicitly enabled. This will affect header search path ordering
for options like `-isystem` when duplicate user paths are present. Should
motivation arise for preserving such suppression of user paths when compiling
with `-fms-compatibility` enabled, it would make sense to introduce a new
option for the `clang-cl` driver to map paths in these environment variabless
to that would match MSVC behavior without impacting other system path options.
---
 clang/docs/ReleaseNotes.rst   |  38 ++
 clang/include/clang/Driver/Options.td |  17 +-
 clang/include/clang/Driver/ToolChain.h|  11 +-
 clang/include/clang/Lex/HeaderSearchOptions.h |  10 +
 clang/lib/Driver/Driver.cpp   |   4 +-
 clang/lib/Driver/ToolChain.cpp|  45 +++
 clang/lib/Driver/ToolChains/Clang.cpp |  11 +-
 clang/lib/Driver/ToolChains/M

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

2024-10-23 Thread Tom Honermann via cfe-commits

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

>From b775b7288035d83281434e27341b98a76623802f Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/5] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically mimicked gcc behavior for header file searching in
which user search paths are ordered before system search paths, user search
paths that duplicate a (later) system search path are ignored, and search
paths that duplicate an earlier search path of the same user/system kind are
ignored.

MSVC behavior differs in that user search paths are ordered before system
search paths (like gcc), and search paths that duplicate an earlier search
path are ignored regardless of user/system kind (similar to gcc, but without
the preference for system search paths over duplicate user search paths).

The gcc and MSVC differences are observable for driver invocations that pass,
e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc behavior will result in
`dir2` being searched before `dir1` while the MSVC behavior will result in
`dir1` being searched before `dir2`.

This patch modifies Clang to match the MSVC behavior for handling of duplicate
header search paths when running in Microsoft compatibility mode (e.g., when
invoked with the `-fms-compatibility` option explicitly or implicitly enabled).
---
 clang/docs/ReleaseNotes.rst   | 14 +++
 clang/lib/Lex/InitHeaderSearch.cpp| 21 ++--
 clang/test/Driver/header-search-duplicates.c  | 76 +++
 .../microsoft-header-search-duplicates.c  | 97 +++
 4 files changed, 199 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/Driver/header-search-duplicates.c
 create mode 100644 clang/test/Driver/microsoft-header-search-duplicates.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1da8c82d52e618..03c27342f31f66 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -638,6 +638,20 @@ Windows Support
   When `-fms-compatibility-version=18.00` or prior is set on the command line 
this Microsoft extension is still
   allowed as VS2013 and prior allow it.
 
+- Clang now matches MSVC behavior for handling of duplicate header search paths
+  when running in Microsoft compatibility mode. Historically, Clang has
+  mimicked gcc behavior in which user search paths are ordered before
+  system search paths, user search paths that duplicate a (later) system search
+  path are ignored, and search paths that duplicate an earlier search path of
+  the same user/system kind are ignored. The MSVC behavior is that user search
+  paths are ordered before system search paths (like gcc), and search paths 
that
+  duplicate an earlier search path are ignored regardless of user/system kind
+  (similar to gcc, but without the preference for system search paths over
+  duplicate user search paths). These differences are observable for driver
+  invocations that pass, e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc
+  behavior will search `dir2` before `dir1` and the MSVC behavior will search
+  `dir1` before `dir2`.
+
 LoongArch Support
 ^
 
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp 
b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..3f487f3a4c1c05 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -368,7 +368,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 /// If there are duplicate directory entries in the specified search list,
 /// remove the later (dead) ones.  Returns the number of non-system headers
 /// removed, which is used to update NumAngled.
-static unsigned RemoveDuplicates(std::vector &SearchList,
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+ std::vector &SearchList,
  unsigned First, bool Verbose) {
   llvm::SmallPtrSet SeenDirs;
   llvm::SmallPtrSet SeenFrameworkDirs;
@@ -394,14 +395,15 @@ static unsigned 
RemoveDuplicates(std::vector &SearchList,
 continue;
 }
 
-// If we have a normal #include dir/framework/headermap that is shadowed
-// later in the chain by a system include location, we actually want to
-// ignore the user's request and drop the user dir... keeping the system
-// dir.  This is weird, but required to emulate GCC's search path 
correctly.
+// When not in MSVC compatibility mode, if we have a normal
+// #include dir/framework/headermap that is shadowed later in the chain by
+// a system include location, we actually want to ignore the user's request
+// and drop the user dir... keeping the system dir.  This is weird, but
+// required to emulate GCC's search path correctly.
 //
 // Since dupes of system dirs are rare, just rescan to find the original
 // that we're nuking instead of using a De

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

2024-10-22 Thread Tom Honermann via cfe-commits

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

>From b775b7288035d83281434e27341b98a76623802f Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/4] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically mimicked gcc behavior for header file searching in
which user search paths are ordered before system search paths, user search
paths that duplicate a (later) system search path are ignored, and search
paths that duplicate an earlier search path of the same user/system kind are
ignored.

MSVC behavior differs in that user search paths are ordered before system
search paths (like gcc), and search paths that duplicate an earlier search
path are ignored regardless of user/system kind (similar to gcc, but without
the preference for system search paths over duplicate user search paths).

The gcc and MSVC differences are observable for driver invocations that pass,
e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc behavior will result in
`dir2` being searched before `dir1` while the MSVC behavior will result in
`dir1` being searched before `dir2`.

This patch modifies Clang to match the MSVC behavior for handling of duplicate
header search paths when running in Microsoft compatibility mode (e.g., when
invoked with the `-fms-compatibility` option explicitly or implicitly enabled).
---
 clang/docs/ReleaseNotes.rst   | 14 +++
 clang/lib/Lex/InitHeaderSearch.cpp| 21 ++--
 clang/test/Driver/header-search-duplicates.c  | 76 +++
 .../microsoft-header-search-duplicates.c  | 97 +++
 4 files changed, 199 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/Driver/header-search-duplicates.c
 create mode 100644 clang/test/Driver/microsoft-header-search-duplicates.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1da8c82d52e618..03c27342f31f66 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -638,6 +638,20 @@ Windows Support
   When `-fms-compatibility-version=18.00` or prior is set on the command line 
this Microsoft extension is still
   allowed as VS2013 and prior allow it.
 
+- Clang now matches MSVC behavior for handling of duplicate header search paths
+  when running in Microsoft compatibility mode. Historically, Clang has
+  mimicked gcc behavior in which user search paths are ordered before
+  system search paths, user search paths that duplicate a (later) system search
+  path are ignored, and search paths that duplicate an earlier search path of
+  the same user/system kind are ignored. The MSVC behavior is that user search
+  paths are ordered before system search paths (like gcc), and search paths 
that
+  duplicate an earlier search path are ignored regardless of user/system kind
+  (similar to gcc, but without the preference for system search paths over
+  duplicate user search paths). These differences are observable for driver
+  invocations that pass, e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc
+  behavior will search `dir2` before `dir1` and the MSVC behavior will search
+  `dir1` before `dir2`.
+
 LoongArch Support
 ^
 
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp 
b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..3f487f3a4c1c05 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -368,7 +368,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 /// If there are duplicate directory entries in the specified search list,
 /// remove the later (dead) ones.  Returns the number of non-system headers
 /// removed, which is used to update NumAngled.
-static unsigned RemoveDuplicates(std::vector &SearchList,
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+ std::vector &SearchList,
  unsigned First, bool Verbose) {
   llvm::SmallPtrSet SeenDirs;
   llvm::SmallPtrSet SeenFrameworkDirs;
@@ -394,14 +395,15 @@ static unsigned 
RemoveDuplicates(std::vector &SearchList,
 continue;
 }
 
-// If we have a normal #include dir/framework/headermap that is shadowed
-// later in the chain by a system include location, we actually want to
-// ignore the user's request and drop the user dir... keeping the system
-// dir.  This is weird, but required to emulate GCC's search path 
correctly.
+// When not in MSVC compatibility mode, if we have a normal
+// #include dir/framework/headermap that is shadowed later in the chain by
+// a system include location, we actually want to ignore the user's request
+// and drop the user dir... keeping the system dir.  This is weird, but
+// required to emulate GCC's search path correctly.
 //
 // Since dupes of system dirs are rare, just rescan to find the original
 // that we're nuking instead of using a De

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

2024-10-22 Thread 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 `) 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 
#include 
#include 

> 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 no

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

2024-10-21 Thread Reid Kleckner via cfe-commits

rnk wrote:

I would stick with `fms-compatibility` being the setting here rather than 
plumbing the driver mode through. Over the years, many build systems have been 
adapted to work with `clang[++]` and `cl` without going through clang-cl.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-21 Thread Tom Honermann via cfe-commits

tahonermann wrote:

@zygoloid,
> On the fourth hand, IIRC the choice of whether header search for a relative 
> path looks in the directory of includers as well as the directory of the 
> current file already depends on `-fms-compatibility`, and this seems sort of 
> similar.

I confirmed the search of the include stack directories is controlled by 
`-fms-compatibility`. The relevant code is 
https://github.com/llvm/llvm-project/blob/f58ce1152703ca753794b8cef36da30bd2668d0f/clang/lib/Lex/PPDirectives.cpp#L1003-L1013.

> So yeah, I dunno what we should do; it doesn't seem clear to me.

Yup. My intuition has been to follow the existing precedent unless/until 
motivation to do otherwise becomes apparent.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-18 Thread Richard Smith via cfe-commits

zygoloid wrote:

I've been pondering this. On the one hand, people switching from the 
`cl.exe`-compatible driver to the GCC-compatible driver might want the MSVC 
interpretation of the path flags. On the other hand, people enabling 
`-fms-compatibility` to accept code written against MSVC in a different 
environment might be surprised if other command-line flags behave differently 
(especially include paths unrelated to those files). On the third hand, this 
only affects cases where the same path is specified in more than one kind of 
list, which seems like it's probably rare. On the fourth hand, IIRC the choice 
of whether header search for a relative path looks in the directory of 
includers as well as the directory of the current file already depends on 
`-fms-compatibility`, and this seems sort of similar. So yeah, I dunno what we 
should do; it doesn't seem clear to me.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-18 Thread Tom Honermann via cfe-commits

tahonermann wrote:

> This choice seems like a question of how the driver's command line is 
> interpreted, rather than a language mode, so I wonder if it makes sense for 
> it to consider `-fms-compatibility` at all or whether this should just be 
> based on the driver mode. Do you have rationale for this also changing under 
> `-fms-compatibility`?

@zygoloid, that is a fair question. My original intent had been to make the 
behavior contingent on driver mode. I switched to tying it to 
`-fms-compatibility` because other preprocessor behavior is contingent on that 
option and because that would be a less invasive change. Duplicate search path 
pruning is currently done by cc1 and I don't think that should be moved to the 
driver. I could be mistaken, but I don't think driver mode is exposed to cc1 
(and that is probably a good thing). We could introduce a new cc1 option to 
opt-in to Microsoft style search path pruning. I'm not opposed to that.

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-18 Thread Tom Honermann via cfe-commits

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

>From b775b7288035d83281434e27341b98a76623802f Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/3] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically mimicked gcc behavior for header file searching in
which user search paths are ordered before system search paths, user search
paths that duplicate a (later) system search path are ignored, and search
paths that duplicate an earlier search path of the same user/system kind are
ignored.

MSVC behavior differs in that user search paths are ordered before system
search paths (like gcc), and search paths that duplicate an earlier search
path are ignored regardless of user/system kind (similar to gcc, but without
the preference for system search paths over duplicate user search paths).

The gcc and MSVC differences are observable for driver invocations that pass,
e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc behavior will result in
`dir2` being searched before `dir1` while the MSVC behavior will result in
`dir1` being searched before `dir2`.

This patch modifies Clang to match the MSVC behavior for handling of duplicate
header search paths when running in Microsoft compatibility mode (e.g., when
invoked with the `-fms-compatibility` option explicitly or implicitly enabled).
---
 clang/docs/ReleaseNotes.rst   | 14 +++
 clang/lib/Lex/InitHeaderSearch.cpp| 21 ++--
 clang/test/Driver/header-search-duplicates.c  | 76 +++
 .../microsoft-header-search-duplicates.c  | 97 +++
 4 files changed, 199 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/Driver/header-search-duplicates.c
 create mode 100644 clang/test/Driver/microsoft-header-search-duplicates.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1da8c82d52e618..03c27342f31f66 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -638,6 +638,20 @@ Windows Support
   When `-fms-compatibility-version=18.00` or prior is set on the command line 
this Microsoft extension is still
   allowed as VS2013 and prior allow it.
 
+- Clang now matches MSVC behavior for handling of duplicate header search paths
+  when running in Microsoft compatibility mode. Historically, Clang has
+  mimicked gcc behavior in which user search paths are ordered before
+  system search paths, user search paths that duplicate a (later) system search
+  path are ignored, and search paths that duplicate an earlier search path of
+  the same user/system kind are ignored. The MSVC behavior is that user search
+  paths are ordered before system search paths (like gcc), and search paths 
that
+  duplicate an earlier search path are ignored regardless of user/system kind
+  (similar to gcc, but without the preference for system search paths over
+  duplicate user search paths). These differences are observable for driver
+  invocations that pass, e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc
+  behavior will search `dir2` before `dir1` and the MSVC behavior will search
+  `dir1` before `dir2`.
+
 LoongArch Support
 ^
 
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp 
b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..3f487f3a4c1c05 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -368,7 +368,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 /// If there are duplicate directory entries in the specified search list,
 /// remove the later (dead) ones.  Returns the number of non-system headers
 /// removed, which is used to update NumAngled.
-static unsigned RemoveDuplicates(std::vector &SearchList,
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+ std::vector &SearchList,
  unsigned First, bool Verbose) {
   llvm::SmallPtrSet SeenDirs;
   llvm::SmallPtrSet SeenFrameworkDirs;
@@ -394,14 +395,15 @@ static unsigned 
RemoveDuplicates(std::vector &SearchList,
 continue;
 }
 
-// If we have a normal #include dir/framework/headermap that is shadowed
-// later in the chain by a system include location, we actually want to
-// ignore the user's request and drop the user dir... keeping the system
-// dir.  This is weird, but required to emulate GCC's search path 
correctly.
+// When not in MSVC compatibility mode, if we have a normal
+// #include dir/framework/headermap that is shadowed later in the chain by
+// a system include location, we actually want to ignore the user's request
+// and drop the user dir... keeping the system dir.  This is weird, but
+// required to emulate GCC's search path correctly.
 //
 // Since dupes of system dirs are rare, just rescan to find the original
 // that we're nuking instead of using a De

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

2024-10-17 Thread Richard Smith via cfe-commits

zygoloid wrote:

This choice seems like a question of how the driver's command line is 
interpreted, rather than a language mode, so I wonder if it makes sense for it 
to consider `-fms-compatibility` at all or whether this should just be based on 
the driver mode. Do you have rationale for this also changing under 
`-fms-compatibility`?

https://github.com/llvm/llvm-project/pull/105738
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-10-17 Thread Tom Honermann via cfe-commits

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

>From b775b7288035d83281434e27341b98a76623802f Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/2] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically mimicked gcc behavior for header file searching in
which user search paths are ordered before system search paths, user search
paths that duplicate a (later) system search path are ignored, and search
paths that duplicate an earlier search path of the same user/system kind are
ignored.

MSVC behavior differs in that user search paths are ordered before system
search paths (like gcc), and search paths that duplicate an earlier search
path are ignored regardless of user/system kind (similar to gcc, but without
the preference for system search paths over duplicate user search paths).

The gcc and MSVC differences are observable for driver invocations that pass,
e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc behavior will result in
`dir2` being searched before `dir1` while the MSVC behavior will result in
`dir1` being searched before `dir2`.

This patch modifies Clang to match the MSVC behavior for handling of duplicate
header search paths when running in Microsoft compatibility mode (e.g., when
invoked with the `-fms-compatibility` option explicitly or implicitly enabled).
---
 clang/docs/ReleaseNotes.rst   | 14 +++
 clang/lib/Lex/InitHeaderSearch.cpp| 21 ++--
 clang/test/Driver/header-search-duplicates.c  | 76 +++
 .../microsoft-header-search-duplicates.c  | 97 +++
 4 files changed, 199 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/Driver/header-search-duplicates.c
 create mode 100644 clang/test/Driver/microsoft-header-search-duplicates.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1da8c82d52e618..03c27342f31f66 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -638,6 +638,20 @@ Windows Support
   When `-fms-compatibility-version=18.00` or prior is set on the command line 
this Microsoft extension is still
   allowed as VS2013 and prior allow it.
 
+- Clang now matches MSVC behavior for handling of duplicate header search paths
+  when running in Microsoft compatibility mode. Historically, Clang has
+  mimicked gcc behavior in which user search paths are ordered before
+  system search paths, user search paths that duplicate a (later) system search
+  path are ignored, and search paths that duplicate an earlier search path of
+  the same user/system kind are ignored. The MSVC behavior is that user search
+  paths are ordered before system search paths (like gcc), and search paths 
that
+  duplicate an earlier search path are ignored regardless of user/system kind
+  (similar to gcc, but without the preference for system search paths over
+  duplicate user search paths). These differences are observable for driver
+  invocations that pass, e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc
+  behavior will search `dir2` before `dir1` and the MSVC behavior will search
+  `dir1` before `dir2`.
+
 LoongArch Support
 ^
 
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp 
b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..3f487f3a4c1c05 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -368,7 +368,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 /// If there are duplicate directory entries in the specified search list,
 /// remove the later (dead) ones.  Returns the number of non-system headers
 /// removed, which is used to update NumAngled.
-static unsigned RemoveDuplicates(std::vector &SearchList,
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+ std::vector &SearchList,
  unsigned First, bool Verbose) {
   llvm::SmallPtrSet SeenDirs;
   llvm::SmallPtrSet SeenFrameworkDirs;
@@ -394,14 +395,15 @@ static unsigned 
RemoveDuplicates(std::vector &SearchList,
 continue;
 }
 
-// If we have a normal #include dir/framework/headermap that is shadowed
-// later in the chain by a system include location, we actually want to
-// ignore the user's request and drop the user dir... keeping the system
-// dir.  This is weird, but required to emulate GCC's search path 
correctly.
+// When not in MSVC compatibility mode, if we have a normal
+// #include dir/framework/headermap that is shadowed later in the chain by
+// a system include location, we actually want to ignore the user's request
+// and drop the user dir... keeping the system dir.  This is weird, but
+// required to emulate GCC's search path correctly.
 //
 // Since dupes of system dirs are rare, just rescan to find the original
 // that we're nuking instead of using a De

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

2024-09-09 Thread Tom Honermann via cfe-commits

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

>From 79b2ceb45cbc4ffb823f56e4c5eedb62f475c780 Mon Sep 17 00:00:00 2001
From: Tom Honermann 
Date: Thu, 22 Aug 2024 09:44:56 -0700
Subject: [PATCH 1/2] [Clang] Match MSVC handling of duplicate header search
 paths in Microsoft compatibility modes.

Clang has historically mimicked gcc behavior for header file searching in
which user search paths are ordered before system search paths, user search
paths that duplicate a (later) system search path are ignored, and search
paths that duplicate an earlier search path of the same user/system kind are
ignored.

MSVC behavior differs in that user search paths are ordered before system
search paths (like gcc), and search paths that duplicate an earlier search
path are ignored regardless of user/system kind (similar to gcc, but without
the preference for system search paths over duplicate user search paths).

The gcc and MSVC differences are observable for driver invocations that pass,
e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc behavior will result in
`dir2` being searched before `dir1` while the MSVC behavior will result in
`dir1` being searched before `dir2`.

This patch modifies Clang to match the MSVC behavior for handling of duplicate
header search paths when running in Microsoft compatibility mode (e.g., when
invoked with the `-fms-compatibility` option explicitly or implicitly enabled).
---
 clang/docs/ReleaseNotes.rst   | 14 +++
 clang/lib/Lex/InitHeaderSearch.cpp| 21 ++--
 clang/test/Driver/header-search-duplicates.c  | 76 +++
 .../microsoft-header-search-duplicates.c  | 97 +++
 4 files changed, 199 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/Driver/header-search-duplicates.c
 create mode 100644 clang/test/Driver/microsoft-header-search-duplicates.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5c156a9c073a9c..d9c96d8902efa9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -363,6 +363,20 @@ Windows Support
   When `-fms-compatibility-version=18.00` or prior is set on the command line 
this Microsoft extension is still
   allowed as VS2013 and prior allow it.
 
+- Clang now matches MSVC behavior for handling of duplicate header search paths
+  when running in Microsoft compatibility mode. Historically, Clang has
+  mimicked gcc behavior in which user search paths are ordered before
+  system search paths, user search paths that duplicate a (later) system search
+  path are ignored, and search paths that duplicate an earlier search path of
+  the same user/system kind are ignored. The MSVC behavior is that user search
+  paths are ordered before system search paths (like gcc), and search paths 
that
+  duplicate an earlier search path are ignored regardless of user/system kind
+  (similar to gcc, but without the preference for system search paths over
+  duplicate user search paths). These differences are observable for driver
+  invocations that pass, e.g., `-Idir1 -isystem dir2 -isystem dir1`. The gcc
+  behavior will search `dir2` before `dir1` and the MSVC behavior will search
+  `dir1` before `dir2`.
+
 LoongArch Support
 ^
 
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp 
b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..3f487f3a4c1c05 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -368,7 +368,8 @@ void InitHeaderSearch::AddDefaultIncludePaths(
 /// If there are duplicate directory entries in the specified search list,
 /// remove the later (dead) ones.  Returns the number of non-system headers
 /// removed, which is used to update NumAngled.
-static unsigned RemoveDuplicates(std::vector &SearchList,
+static unsigned RemoveDuplicates(const LangOptions &Lang,
+ std::vector &SearchList,
  unsigned First, bool Verbose) {
   llvm::SmallPtrSet SeenDirs;
   llvm::SmallPtrSet SeenFrameworkDirs;
@@ -394,14 +395,15 @@ static unsigned 
RemoveDuplicates(std::vector &SearchList,
 continue;
 }
 
-// If we have a normal #include dir/framework/headermap that is shadowed
-// later in the chain by a system include location, we actually want to
-// ignore the user's request and drop the user dir... keeping the system
-// dir.  This is weird, but required to emulate GCC's search path 
correctly.
+// When not in MSVC compatibility mode, if we have a normal
+// #include dir/framework/headermap that is shadowed later in the chain by
+// a system include location, we actually want to ignore the user's request
+// and drop the user dir... keeping the system dir.  This is weird, but
+// required to emulate GCC's search path correctly.
 //
 // Since dupes of system dirs are rare, just rescan to find the original
 // that we're nuking instead of using a De