[clang] [llvm] [Clang] Match MSVC handling of duplicate header search paths in Microsoft compatibility modes. (PR #105738)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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