[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 105671.
mibintc added a comment.
I updated the patch as James directed, moving the preinclude of stdc-predef.h
into clang.cpp and out of the Linux toolchain entirely. I also needed to
update a couple more tests in tools/extra, so I will need to update that
related differential as well.
https://reviews.llvm.org/D34158
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/PreprocessorOptions.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/InitPreprocessor.cpp
test/Driver/Inputs/stdc-predef/bin/.keep
test/Driver/Inputs/stdc-predef/usr/i386-unknown-linux/lib/.keep
test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
test/Driver/Inputs/stdc-predef/usr/lib/.keep
test/Driver/Inputs/stdc-predef/usr/x86_64-unknown-linux/lib/.keep
test/Driver/clang_cpp.c
test/Driver/crash report spaces.c
test/Driver/crash-report-header.h
test/Driver/crash-report.c
test/Driver/rewrite-legacy-objc.m
test/Driver/rewrite-map-in-diagnostics.c
test/Driver/rewrite-objc.m
test/Driver/stdc-predef-not.c
test/Driver/stdc-predef.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
test/Preprocessor/ignore-pragmas.c
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3175,6 +3175,13 @@
KernelOrKext) {
CmdArgs.push_back("-ffreestanding");
IsHosted = false;
+ } else {
+if (!Args.hasArg(options::OPT_nostdinc)) {
+ // For standards compliance, clang will preinclude
+ // -ffreestanding suppresses this behavior.
+ CmdArgs.push_back("-finclude-if-exists");
+ CmdArgs.push_back("stdc-predef.h");
+}
}
// Forward -f (flag) options which we can pass directly.
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2425,6 +2425,10 @@
for (const Arg *A : Args.filtered(OPT_chain_include))
Opts.ChainedIncludes.emplace_back(A->getValue());
+ // Add the ordered list of -finclude-if-exists.
+ for (const Arg *A : Args.filtered(OPT_finclude_if_exists))
+Opts.FIncludeIfExists.emplace_back(A->getValue());
+
for (const Arg *A : Args.filtered(OPT_remap_file)) {
std::pair Split = StringRef(A->getValue()).split(';');
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -69,6 +69,12 @@
Builder.append(Twine("#include \"") + File + "\"");
}
+static void AddImplicitIncludeIfExists(MacroBuilder , StringRef File) {
+ Builder.append(Twine("#if __has_include( \"") + File + "\")");
+ Builder.append(Twine("#include \"") + File + "\"");
+ Builder.append(Twine("#endif"));
+}
+
static void AddImplicitIncludeMacros(MacroBuilder , StringRef File) {
Builder.append(Twine("#__include_macros \"") + File + "\"");
// Marker token to stop the __include_macros fetch loop.
@@ -1088,6 +1094,12 @@
// Exit the command line and go back to (2 is LC_LEAVE).
if (!PP.getLangOpts().AsmPreprocessor)
Builder.append("# 1 \"\" 2");
+
+ // Process -finclude-if-exists directives.
+ for (unsigned i = 0, e = InitOpts.FIncludeIfExists.size(); i != e; ++i) {
+const std::string = InitOpts.FIncludeIfExists[i];
+AddImplicitIncludeIfExists(Builder, Path);
+ }
// If -imacros are specified, include them now. These are processed before
// any -include directives.
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -52,6 +52,7 @@
/// \brief Runs the current AST visitor over the given code.
bool runOver(StringRef Code, Language L = Lang_CXX) {
std::vector Args;
+Args.push_back("-nostdinc");
switch (L) {
case Lang_C: Args.push_back("-std=c99"); break;
case Lang_CXX98: Args.push_back("-std=c++98"); break;
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -729,6 +729,8 @@
HelpText<"Use specified token cache file">;
def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">,
HelpText<"include a detailed record of preprocessing actions">;
+def finclude_if_exists : Separate<["-"], "finclude-if-exists">, MetaVarName<"">,
+ HelpText<"Include file before parsing if file exists">;
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
jyknight added inline comments.
Comment at: lib/Driver/ToolChains/Linux.cpp:755
+!Version.isOlderThan(4, 8, 0)) {
+ // For gcc >= 4.8.x, clang will preinclude
+ // -ffreestanding suppresses this behavior.
jyknight wrote:
> I don't see why it makes any sense to condition this based on a GCC
> installation existing and being >= 4.8, since this header comes from libc,
> and is necessary for standards compliance even if there's absolutely no GCC
> installed or involved.
>
> Additionally, something like this -- a way for the libc to be involved in
> setting up predefined macros -- seems probably necessary for *any* libc, if
> they want to be strictly standards-compliant. Some of the
> required-to-be-predefined macros like `__STDC_ISO_10646__` can only really be
> provided by the libc, since the appropriate value is dependent on the locale
> implementation in the libc, and not on the compiler at all.
>
> It's possible that glibc on Linux is the only one who's cared to implement it
> thus far (and, only when you're using GCC, at that, hence this change...).
> That seems likely, really, since mostly the impacted macros are nearly
> useless, so who cares... :) However, if others want to be conforming, I
> expect they _ought_ to be using this mechanism, as well...
>
> Thus, perhaps it ought to be an unconditional behavior of clang to include
> the file if it exists, unless `-ffreestanding` is passed.
This is still checking GCCInstallation.isValid(), OPT_nostdinc, and only
attempting to load the header on Linux OSes.
I don't think it shouldn't be conditioned by the former two, and I think it
should attempt to include stdc-predef.h if it exists on *all* OSes, not just
Linux (for the reasons described in the above comment).
https://reviews.llvm.org/D34158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 105526.
mibintc added a comment.
I rewrote the patch the way that Richard suggested, adding a cc1 option
"finclude-if-exists" and injecting #if __has_include etc. [OK to use finclude?
include-if-exists preferred?]
I responded to James' remarks and removed the gcc version check.
Does this look better?
https://reviews.llvm.org/D34158
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/PreprocessorOptions.h
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/InitPreprocessor.cpp
test/Driver/Inputs/stdc-predef/bin/.keep
test/Driver/Inputs/stdc-predef/usr/i386-unknown-linux/lib/.keep
test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
test/Driver/Inputs/stdc-predef/usr/lib/.keep
test/Driver/Inputs/stdc-predef/usr/x86_64-unknown-linux/lib/.keep
test/Driver/clang_cpp.c
test/Driver/crash report spaces.c
test/Driver/crash-report-header.h
test/Driver/crash-report.c
test/Driver/gcc-predef-not.c
test/Driver/gcc-predef.c
test/Driver/rewrite-map-in-diagnostics.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
test/Preprocessor/ignore-pragmas.c
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+ AddGnuIncludeArgs(DriverArgs, CC1Args);
}
static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,20 @@
return "";
}
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid()) {
+if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+!DriverArgs.hasArg(clang::driver::options::OPT_nostdinc)) {
+ // For gcc compatibility, clang will preinclude
+ // -ffreestanding suppresses this behavior.
+ CC1Args.push_back("-finclude-if-exists");
+ CC1Args.push_back("stdc-predef.h");
+}
+ }
+}
+
+
void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const {
// We need a detected GCC installation on Linux to provide libstdc++'s
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2425,6 +2425,10 @@
for (const Arg *A : Args.filtered(OPT_chain_include))
Opts.ChainedIncludes.emplace_back(A->getValue());
+ // Add the ordered list of -finclude-if-exists.
+ for (const Arg *A : Args.filtered(OPT_finclude_if_exists))
+Opts.FIncludeIfExists.emplace_back(A->getValue());
+
for (const Arg *A : Args.filtered(OPT_remap_file)) {
std::pair Split = StringRef(A->getValue()).split(';');
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -69,6 +69,12 @@
Builder.append(Twine("#include \"") + File + "\"");
}
+static void AddImplicitIncludeIfExists(MacroBuilder , StringRef File) {
+ Builder.append(Twine("#if __has_include( \"") + File + "\")");
+ Builder.append(Twine("#include \"") + File + "\"");
+ Builder.append(Twine("#endif"));
+}
+
static void AddImplicitIncludeMacros(MacroBuilder , StringRef File) {
Builder.append(Twine("#__include_macros \"") + File + "\"");
// Marker token to stop the __include_macros fetch loop.
@@ -1088,6 +1094,12 @@
// Exit the command line and go back to (2 is LC_LEAVE).
if (!PP.getLangOpts().AsmPreprocessor)
Builder.append("# 1 \"\" 2");
+
+ // Process -finclude-if-exists directives.
+ for (unsigned i = 0, e = InitOpts.FIncludeIfExists.size(); i != e; ++i) {
+const std::string = InitOpts.FIncludeIfExists[i];
+AddImplicitIncludeIfExists(Builder, Path);
+ }
// If -imacros are
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
jyknight added inline comments.
Comment at: lib/Driver/ToolChains/Linux.cpp:755
+!Version.isOlderThan(4, 8, 0)) {
+ // For gcc >= 4.8.x, clang will preinclude
+ // -ffreestanding suppresses this behavior.
I don't see why it makes any sense to condition this based on a GCC
installation existing and being >= 4.8, since this header comes from libc, and
is necessary for standards compliance even if there's absolutely no GCC
installed or involved.
Additionally, something like this -- a way for the libc to be involved in
setting up predefined macros -- seems probably necessary for *any* libc, if
they want to be strictly standards-compliant. Some of the
required-to-be-predefined macros like `__STDC_ISO_10646__` can only really be
provided by the libc, since the appropriate value is dependent on the locale
implementation in the libc, and not on the compiler at all.
It's possible that glibc on Linux is the only one who's cared to implement it
thus far (and, only when you're using GCC, at that, hence this change...). That
seems likely, really, since mostly the impacted macros are nearly useless, so
who cares... :) However, if others want to be conforming, I expect they _ought_
to be using this mechanism, as well...
Thus, perhaps it ought to be an unconditional behavior of clang to include the
file if it exists, unless `-ffreestanding` is passed.
https://reviews.llvm.org/D34158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
On 5 July 2017 at 14:09, Melanie Blower via Phabricator < revi...@reviews.llvm.org> wrote: > mibintc added a comment. > > Jonas asked about adding a new test to ensure that "-include > stdc-predef.h" does not get added if the file doesn't exist. > > I added a reply to that but I can't see where it went. So I'm writing the > reply again. > > The current version of the patch doesn't check for the existence of > stdc-predef.h. As far as I understand, this is consistent with gcc > behavior. It is expected that if you are on a system without stdc-predef.h > then you can add -ffreestanding. > That does not seem reasonable. Plenty of libc implementations exist that don't provide this header. GCC might want to strongly express a preference for glibc, but we don't. > In a prior revision of this patch (see Diff 4), I attempted to iterate > through the system directories to check for the existence of the file > before adding -include with a full path name. However that patch didn't > work. I was using this call to iterate through the system includes: > getAllArgValues(options::OPT_isystem). However, the directory where > stdc-predef.h is located, /usr/include, is added into the search path by > using the option -internal-externc-isystem. > getAllArgValues(options::OPT_isystem) > does not iterate through the include directories which were added via > -internal-externc-isystem. [Note: is this a bug?]. There is no enumeration > value within options::OPT_? which corresponds to -internal-externc-isystem. > > I could check for the existence of this file: /usr/include/stdc-predef.h; > I don't know whether this would be acceptable or if it's correct. Trying to fake up headers search from the driver doesn't sound like a good way forward. Perhaps instead you could add a -include-if-exists flag to -cc1 for this purpose, where -include-if-exists X would expand to something like #if __has_include(X) #include X #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc added a comment. Jonas asked about adding a new test to ensure that "-include stdc-predef.h" does not get added if the file doesn't exist. I added a reply to that but I can't see where it went. So I'm writing the reply again. The current version of the patch doesn't check for the existence of stdc-predef.h. As far as I understand, this is consistent with gcc behavior. It is expected that if you are on a system without stdc-predef.h then you can add -ffreestanding. In a prior revision of this patch (see Diff 4), I attempted to iterate through the system directories to check for the existence of the file before adding -include with a full path name. However that patch didn't work. I was using this call to iterate through the system includes: getAllArgValues(options::OPT_isystem). However, the directory where stdc-predef.h is located, /usr/include, is added into the search path by using the option -internal-externc-isystem. getAllArgValues(options::OPT_isystem) does not iterate through the include directories which were added via -internal-externc-isystem. [Note: is this a bug?]. There is no enumeration value within options::OPT_? which corresponds to -internal-externc-isystem. I could check for the existence of this file: /usr/include/stdc-predef.h; I don't know whether this would be acceptable or if it's correct. https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 105329.
mibintc added a comment.
I responded to the review from Jonas Hahnfeld: I moved the entire change into
ToolChains/Linux and removed the modifications to Gnu.h and Gnu.cpp; I modified
the new tests to use -### with FileCheck, and added a tree in Inputs/ for the
new tests. Jonas please see inline reply concerning whether to--or how
to--check for existence of stdc-predef.h
Also, note that there are some tests from clang-tidy that need to be updated. I
have a separate patch for that since it's in a different repo. I assume it
would be good to get those modifications landed before this patch.
https://reviews.llvm.org/D34158
Files:
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
test/Driver/Inputs/stdc-predef/bin/.keep
test/Driver/Inputs/stdc-predef/usr/i386-unknown-linux/lib/.keep
test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
test/Driver/Inputs/stdc-predef/usr/lib/.keep
test/Driver/Inputs/stdc-predef/usr/x86_64-unknown-linux/lib/.keep
test/Driver/clang_cpp.c
test/Driver/crash-report.c
test/Driver/gcc-predef-not.c
test/Driver/gcc-predef.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+ AddGnuIncludeArgs(DriverArgs, CC1Args);
}
static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,22 @@
return "";
}
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid()) {
+const Generic_GCC::GCCVersion = GCCInstallation.getVersion();
+if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+!DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+!Version.isOlderThan(4, 8, 0)) {
+ // For gcc >= 4.8.x, clang will preinclude
+ // -ffreestanding suppresses this behavior.
+ CC1Args.push_back("-include");
+ CC1Args.push_back("stdc-predef.h");
+}
+ }
+}
+
+
void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const {
// We need a detected GCC installation on Linux to provide libstdc++'s
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -52,6 +52,7 @@
/// \brief Runs the current AST visitor over the given code.
bool runOver(StringRef Code, Language L = Lang_CXX) {
std::vector Args;
+Args.push_back("-nostdinc");
switch (L) {
case Lang_C: Args.push_back("-std=c99"); break;
case Lang_CXX98: Args.push_back("-std=c++98"); break;
Index: test/Driver/gcc-predef.c
===
--- test/Driver/gcc-predef.c
+++ test/Driver/gcc-predef.c
@@ -0,0 +1,8 @@
+// Test that clang preincludes stdc-predef.h on Linux with gcc installations
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: --gcc-toolchain="" --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck -check-prefix CHECK-PREDEF %s
+
+// CHECK-PREDEF: "-include" "stdc-predef.h"
+int i;
Index: test/Driver/gcc-predef-not.c
===
--- test/Driver/gcc-predef-not.c
+++ test/Driver/gcc-predef-not.c
@@ -0,0 +1,7 @@
+// Test that -ffreestanding suppresses the preinclude of stdc-predef.h.
+//
+// RUN: %clang %s -### -c -ffreestanding 2>&1 \
+// RUN: --gcc-toolchain="" --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not '"-include" "stdc-predef.h"' %s
+
+int i;
Index: test/Driver/crash-report.c
===
--- test/Driver/crash-report.c
+++ test/Driver/crash-report.c
@@ -2,7 +2,7 @@
// RUN: mkdir %t
// RUN: not env TMPDIR=%t TEMP=%t
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc added a comment. In https://reviews.llvm.org/D34158#797968, @Hahnfeld wrote: > In https://reviews.llvm.org/D34158#797170, @mibintc wrote: > > > The other test that fails is my own new test! It fails because I don't know > > how to set it up so the test thinks it has a gcc toolchain with version > > > 4.8. I tried using gcc-toolchain set to various other Linux toolchains that > > i see in the test/Driver/Inputs - none of them cause the gcc version to be > > in the range. I also tried using -ccc-installation=Inputs/ which I see > > being used for gcc version parsing. How can I set up the test so that the > > GCCInstallation has a Version >= 4.8? I test the new functionality from > > the console on Linux and can confirm it's working. > > > You could try adding a completely new directory in `Inputs/`. Add the > directory `usr/lib/gcc/x86_64-unknown-linux/4.9.0` underneath which > //should// be recognized as the version. Maybe you have to add some more > files, I'm not really familiar with the detection mechanism... Thanks. I did that by copying the directory from fake_install_tree into a new install directory named stdc-predef. However, this modification didn't result in altering the macro values for gnu-major and gnu-minor. On the other hand, it is necessary to have a gcc installation in the --sysroot directory. The test didn't pass if the gcc installation wasn't present in the Inputs directory. I used the method you suggested and modified the test to use -### and check for the presence or absence of -include stdc-predef.h. Now these new tests are passing. Thank you very much for the suggestion. Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
ilya-biryukov added a comment. Here's a patch to fix ClangdTests: https://reviews.llvm.org/D34936 We should probably land it before your patch to avoid ClangdTests failures between those llvm and clang-tools-extra revisions. Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
Hahnfeld added a comment.
In https://reviews.llvm.org/D34158#797170, @mibintc wrote:
> The other test that fails is my own new test! It fails because I don't know
> how to set it up so the test thinks it has a gcc toolchain with version >
> 4.8. I tried using gcc-toolchain set to various other Linux toolchains that i
> see in the test/Driver/Inputs - none of them cause the gcc version to be in
> the range. I also tried using -ccc-installation=Inputs/ which I see being
> used for gcc version parsing. How can I set up the test so that the
> GCCInstallation has a Version >= 4.8? I test the new functionality from the
> console on Linux and can confirm it's working.
You could try adding a completely new directory in `Inputs/`. Add the directory
`usr/lib/gcc/x86_64-unknown-linux/4.9.0` underneath which //should// be
recognized as the version. Maybe you have to add some more files, I'm not
really familiar with the detection mechanism...
Comment at: lib/Driver/ToolChains/Gnu.cpp:2332-2343
+void Generic_GCC::addGnuIncludeArgs(const ArgList ,
+ArgStringList ) const {
+ const Generic_GCC::GCCVersion = GCCInstallation.getVersion();
+ if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+ !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+ !Version.isOlderThan(4, 8, 0)) {
+// For gcc >= 4.8.x, gcc will preinclude
I think this can still be moved to Linux...
Comment at: test/Driver/gcc-predef.c:27
+#if defined( DUMMY_STDC_PREDEF )
+ #error "stdc-predef.h should not be preincluded for gcc < 4.8.x"
+#endif
Maybe it's better to run clang with `-###` and checking that it passes
`-include` to cc1?
Then you can have:
```
// CHECK-PREDEF: "-include" "stdc-predef.h"
// CHECK-NO-PREDEF-NOT: "-include" "stdc-predef.h"
```
and pass the corresponding prefixes to FileCheck. Could you also add a test
that it the file is not included if it does not exist? This can be done with
the current `basic_linux_tree`
Repository:
rL LLVM
https://reviews.llvm.org/D34158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 104911.
mibintc added a subscriber: ilya-biryukov.
mibintc added a comment.
The previous (n-1) version didn't work: I wanted to iterate through the system
include directories to check for the existence of stdc-predef.h but clang is
using a different kind of system include option for /usr and /usr/include so
the iterator didn't find any system includes at all, i don't think there is an
iterator which will iterate through these special kind of include directories.
[Kind of embarrassing that I posted that patch which didn't work at all, sorry
about that.]
This one behaves more like gcc anyway. gcc puts the -include option in there
regardless, and to suppress the behavior you are to use the -ffreestanding
option.
I responded to the remarks from Jonas Hahnfeld, making the patch only for Linux.
I'm having trouble with one of the existing tests. These are the ones that
fail. I don't know how to fix the text. It's a google test and it is using a
dummy file system with whitelisted directories. @ilya-biryukov Is there a way
to pass in a command line argument? if so, I could fix it these failures by
adding -ffreestanding option which suppresses the new behavior. That's how I
fixed the many test failures which I encountered with this new behavior.
Extra Tools Unit Tests :: clangd/ClangdTests/ClangdVFSTest.Parse
Extra Tools Unit Tests :: clangd/ClangdTests/ClangdVFSTest.ParseWithHeader
Extra Tools Unit Tests :: clangd/ClangdTests/ClangdVFSTest.Reparse
The other test that fails is my own new test! It fails because I don't know how
to set it up so the test thinks it has a gcc toolchain with version > 4.8. I
tried using gcc-toolchain set to various other Linux toolchains that i see in
the test/Driver/Inputs - none of them cause the gcc version to be in the range.
I also tried using -ccc-installation=Inputs/ which I see being used for gcc
version parsing. How can I set up the test so that the GCCInstallation has a
Version >= 4.8? I test the new functionality from the console on Linux and can
confirm it's working.
Failing Tests (4): -- the 3 listed above, as well as
Clang :: Driver/gcc-predef.c -- my new test, the 3rd run which confirms that
the include is added
Repository:
rL LLVM
https://reviews.llvm.org/D34158
Files:
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Gnu.h
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
test/Driver/Inputs/basic_linux_tree/usr/include/stdc-predef.h
test/Driver/clang_cpp.c
test/Driver/crash-report.c
test/Driver/gcc-predef.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -2329,6 +2329,19 @@
}
}
+void Generic_GCC::addGnuIncludeArgs(const ArgList ,
+ArgStringList ) const {
+ const Generic_GCC::GCCVersion = GCCInstallation.getVersion();
+ if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+ !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+ !Version.isOlderThan(4, 8, 0)) {
+// For gcc >= 4.8.x, gcc will preinclude
+// -ffreestanding suppresses this behavior.
+CC1Args.push_back("-include");
+CC1Args.push_back("stdc-predef.h");
+ }
+}
+
void Generic_GCC::AddClangCXXStdlibIncludeArgs(const ArgList ,
ArgStringList ) const {
if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+ AddGnuIncludeArgs(DriverArgs, CC1Args);
}
static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,13 @@
return "";
}
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc planned changes to this revision. mibintc added a comment. I'm planning to rework this patch again. sorry for the churn. Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
Hahnfeld added a subscriber: cfe-commits.
Hahnfeld edited reviewers, added: rsmith, rengolin; removed: cfe-commits.
Hahnfeld added a comment.
Some comments inline. In general you should consider posting an RFC on cfe-dev
because this change will basically affect all compilations on GNU/Linux if the
file is present.
Adding Richard (general maintainer) and Renato (ARM Linux) so they are aware.
Comment at: include/clang/Driver/ToolChain.h:459-462
+ /// \brief Add arguments to use system-specific GNU includes.
+ virtual void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
Why do you need to define this method in the generic `ToolChain`?
Comment at: lib/Driver/ToolChains/Gnu.cpp:2332-2349
+void Generic_GCC::addGnuIncludeArgs(const ArgList ,
+ArgStringList ) const {
+ const Generic_GCC::GCCVersion = GCCInstallation.getVersion();
+ if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+ !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+ !Version.isOlderThan(4, 8, 0)) {
+// If stdc-predef.h exists in the sytem includes, then -include it.
If this is GNU/Linux only, why not move to the `Linux` toolchain entirely?
Comment at: lib/Driver/ToolChains/Gnu.h:328-330
+ void addGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
This starts with a lower-case character and won't override the method in
`ToolChain`. Adding the keyword `override` should reveal this mistake.
Comment at: test/Driver/gcc-predef.c:1
+// Test that gcc preincludes stdc-predef.h on Linux
+//
s/gcc/clang/ ?
Comment at: test/Driver/gcc-predef.c:9
+ #else
+#if !defined( _STDC_PREDEF_H )
+ #error "stdc-predef.h should be preincluded for GNU/Linux 4.8 and higher"
The driver will only include `stdc-predef.h` if it can be found in the system.
With that, the current version of this test will only run on such Linux system.
Maybe add a basic tree in `test/Driver/Inputs` and test that the corresponding
header is included?
Comment at: test/Driver/gcc-predef.c:14-17
+// Test that gcc preincludes stdc-predef.h on Linux
+//
+// RUN: %clang -c --target=i386-unknown-linux %s -Xclang -verify
+// expected-no-diagnostics
What's the difference to the test above?
Repository:
rL LLVM
https://reviews.llvm.org/D34158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
fedor.sergeev added a comment. > can you recommend someone to review the extra test changes? I'm rather new to the project and thus rather bad at finding reviewers. Also generally I do not consider myself qualified enough to be the only reviewer, though in this particular case I believe I spent enough time on the toolchain code for my own Solaris work. I feel less qualified for the tests section... I will take a look at the final version tomorrow. Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc added a comment. @fedor.sergeev do you have time to review my (hopefully) final revision? Also can you recommend someone to review the extra test changes? Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 103871.
mibintc added a comment.
Here's a modified revision which checks if stdc-predef.h exists before adding
a -include option to that file. this is only for Linux using gcc 4.8 and
higher. Several tests needed to be fixed because of this change - i fixed them
by suppressing the preinclude using ffreestanding or nostdinc. Also i need to
change about 10 clang/tools/extra/test/clang-tidy tests. Those test changes
aren't in this patch. Please tell me: what's the correct procedure for getting
those tests reviewed? Thanks Fedor for your reviews!
--Melanie
Repository:
rL LLVM
https://reviews.llvm.org/D34158
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Gnu.h
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
test/Driver/clang_cpp.c
test/Driver/crash-report.c
test/Driver/gcc-predef.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -2329,6 +2329,24 @@
}
}
+void Generic_GCC::addGnuIncludeArgs(const ArgList ,
+ArgStringList ) const {
+ const Generic_GCC::GCCVersion = GCCInstallation.getVersion();
+ if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+ !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+ !Version.isOlderThan(4, 8, 0)) {
+// If stdc-predef.h exists in the sytem includes, then -include it.
+for (const auto Path : DriverArgs.getAllArgValues(options::OPT_isystem)) {
+ const auto FilePath = Path + "stdc-predef.h";
+ if (llvm::sys::fs::exists(Path)) {
+CC1Args.push_back("-include");
+CC1Args.push_back(FilePath.c_str());
+break;
+ }
+}
+ }
+}
+
void Generic_GCC::AddClangCXXStdlibIncludeArgs(const ArgList ,
ArgStringList ) const {
if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+ AddGnuIncludeArgs(DriverArgs, CC1Args);
}
static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,13 @@
return "";
}
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid())
+addGnuIncludeArgs(DriverArgs, CC1Args);
+}
+
+
void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const {
// We need a detected GCC installation on Linux to provide libstdc++'s
Index: lib/Driver/ToolChains/Gnu.h
===
--- lib/Driver/ToolChains/Gnu.h
+++ lib/Driver/ToolChains/Gnu.h
@@ -325,6 +325,9 @@
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const;
+ void addGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
/// @}
private:
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -716,6 +716,9 @@
return Res;
}
+void ToolChain::AddGnuIncludeArgs(const ArgList ,
+ ArgStringList ) const {}
+
void ToolChain::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {}
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -52,6 +52,7 @@
/// \brief Runs the current AST visitor over the given
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc abandoned this revision. mibintc added a comment. I want to submit a modified patch Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc planned changes to this revision. mibintc added a comment. I'm submitting another revision which checks for the existence of stdc-predef.h before inserting the -include option. Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc added a comment. In https://reviews.llvm.org/D34158#782467, @fedor.sergeev wrote: > LGTM wrt your update to sources. > And sorry, I'm not that qualified to answer your question on failing tests. > > Probing existence of this header would make a sense, yet you are including it > w/o a full path, so how are you going to find it for this probe? Yes, I'd have to change this patch to iterate through the system headers. Once I found an instance I'd keep the full path name and -include it using full path name. I'll go ahead and change the patch to do this. The other thought I had is: don't check for gcc version -- just add this functionality for all gcc versions on Linux. The reason I'd do that is to avoid creating lit test failures when the lit tests are running with gcc versions less than gcc 4.8; thus far I've been able to fix the lit failures by adding "-nostdinc". Adding "-nostdinc" to the tests would work regardless of gcc version. But I'm concerned that I will need to update some of the lit tests by checking for different output. In that case the expected output would differ based on gcc version, and I don't know if it's possible to express "don't run this lit test for gcc< 4.8". Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
fedor.sergeev added a comment. LGTM wrt your update to sources. And sorry, I'm not that qualified to answer your question on failing tests. Probing existence of this header would make a sense, yet you are including it w/o a full path, so how are you going to find it for this probe? Repository: rL LLVM https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 102605.
mibintc added a comment.
Thanks for your review, Fedor. I moved AddGnuIncludeArgs out of
GCCInstallation, like you suggested, and put it along where the library
includes are added. I also pulled out the change for MipsLinux since I don't
have a way to verify that.
I still have the same 7 test failures when make check-all on Linux. 3 of the
fails are from "clangd" unit tests. Not sure why they are failing but i scanned
the tests a little and it talks about access only to whitelisted directories,
so I assume they aren't working because the dir where std-predef.h lives is not
white listed. Maybe I should be sure that "file exists(stdc-predef.h)", e.g.
using the system include path, before adding the -include into the command
line--what do you think?
Repository:
rL LLVM
https://reviews.llvm.org/D34158
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Gnu.h
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
test/Driver/clang_cpp.c
test/Driver/gcc-predef.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -2329,6 +2329,17 @@
}
}
+void Generic_GCC::addGnuIncludeArgs(const ArgList ,
+ArgStringList ) const {
+ const Generic_GCC::GCCVersion = GCCInstallation.getVersion();
+ if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+ !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+ !Version.isOlderThan(4, 8, 0)) {
+CC1Args.push_back("-include");
+CC1Args.push_back("stdc-predef.h");
+ }
+}
+
void Generic_GCC::AddClangCXXStdlibIncludeArgs(const ArgList ,
ArgStringList ) const {
if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+ AddGnuIncludeArgs(DriverArgs, CC1Args);
}
static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,13 @@
return "";
}
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid())
+addGnuIncludeArgs(DriverArgs, CC1Args);
+}
+
+
void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const {
// We need a detected GCC installation on Linux to provide libstdc++'s
Index: lib/Driver/ToolChains/Gnu.h
===
--- lib/Driver/ToolChains/Gnu.h
+++ lib/Driver/ToolChains/Gnu.h
@@ -325,6 +325,9 @@
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const;
+ void addGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+
/// @}
private:
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -716,6 +716,9 @@
return Res;
}
+void ToolChain::AddGnuIncludeArgs(const ArgList ,
+ ArgStringList ) const {}
+
void ToolChain::AddCudaIncludeArgs(const ArgList ,
ArgStringList ) const {}
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -52,6 +52,7 @@
/// \brief Runs the current AST visitor over the given code.
bool runOver(StringRef Code, Language L = Lang_CXX) {
std::vector Args;
+
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
fedor.sergeev added a comment. > docs are rather clear that this functionality is Linux-only Oh, well, gcc implementation is rather generic, it relies on gcc's config.gcc to detect glibc and then it adds a hook that unconditionally provides stdc-predef.h (TARGET_C_PREINCLUDE). Currently config.gcc does that for the following pattern of triples: *-*-linux* | frv-*-*linux* | *-*-kfreebsd*-gnu | *-*-knetbsd*-gnu | *-*-gnu* | *-*-kopensolaris*-gnu) I'm not sure how far into that do you really want to dive :-/ https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
fedor.sergeev added inline comments. Comment at: lib/Driver/ToolChains/Gnu.h:219-220 +void AddGnuIncludeArgs(const llvm::opt::ArgList , + llvm::opt::ArgStringList ) const; + To me this does not belong to GCC-Installation-Detector at all, as it does not have anything to do with detection. As I see the only reason for adding it here is to check Version vs 4.8.0. If thats true then adding getVersion() here and using it elsewhere seems a better solution. https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
fedor.sergeev added a comment. There is no /usr/include/stdc-predef.h on Solaris and I see no sense in adding this -include for anything except Linux (as docs are rather clear that this functionality is Linux-only). Checked gcc on Solaris11/12 both SPARC and X86 - none are doing stdc-predef.h inclusion (checking with gcc - -dI https://reviews.llvm.org/D34158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc updated this revision to Diff 102382.
mibintc added a comment.
forgot to add the new lit test
https://reviews.llvm.org/D34158
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Gnu.h
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
lib/Driver/ToolChains/MipsLinux.cpp
lib/Driver/ToolChains/MipsLinux.h
lib/Driver/ToolChains/Solaris.cpp
test/Driver/clang_cpp.c
test/Driver/gcc-predef.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/MipsLinux.h
===
--- lib/Driver/ToolChains/MipsLinux.h
+++ lib/Driver/ToolChains/MipsLinux.h
@@ -28,6 +28,8 @@
void
AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const override;
CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList ) const override;
Index: lib/Driver/ToolChains/Solaris.cpp
===
--- lib/Driver/ToolChains/Solaris.cpp
+++ lib/Driver/ToolChains/Solaris.cpp
@@ -189,5 +189,6 @@
"." + Version.MinorStr + "/include/c++/" +
Version.Text + "/" +
GCCInstallation.getTriple().str());
+GCCInstallation.AddGnuIncludeArgs(DriverArgs, CC1Args);
}
}
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/MipsLinux.cpp
===
--- lib/Driver/ToolChains/MipsLinux.cpp
+++ lib/Driver/ToolChains/MipsLinux.cpp
@@ -64,6 +64,12 @@
}
}
+void MipsLLVMToolChain::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid())
+GCCInstallation.AddGnuIncludeArgs(DriverArgs, CC1Args);
+}
+
Tool *MipsLLVMToolChain::buildLinker() const {
return new tools::gnutools::Linker(*this);
}
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1771,6 +1771,16 @@
return false;
}
+void Generic_GCC::GCCInstallationDetector::AddGnuIncludeArgs(
+const ArgList , ArgStringList ) const {
+ if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+ !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+ !Version.isOlderThan(4, 8, 0)) {
+CC1Args.push_back("-include");
+CC1Args.push_back("stdc-predef.h");
+ }
+}
+
/*static*/ void Generic_GCC::GCCInstallationDetector::CollectLibDirsAndTriples(
const llvm::Triple , const llvm::Triple ,
SmallVectorImpl ,
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+ GCCInstallation.AddGnuIncludeArgs(DriverArgs, CC1Args);
}
static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,13 @@
return "";
}
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid())
+GCCInstallation.AddGnuIncludeArgs(DriverArgs, CC1Args);
+}
+
+
void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const {
// We need a detected GCC installation on Linux to provide libstdc++'s
Index: lib/Driver/ToolChains/Gnu.h
===
--- lib/Driver/ToolChains/Gnu.h
+++ lib/Driver/ToolChains/Gnu.h
@@ -216,6 +216,9 @@
/// \brief Check whether we detected a valid GCC install.
bool isValid() const { return IsValid; }
+void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+
[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude
mibintc created this revision.
mibintc created this object with visibility "All Users".
Herald added subscribers: fedor.sergeev, arichardson, Anastasia, sdardis,
klimek.
As reported in llvm bugzilla 32377.
Here’s a patch to add preinclude of stdc-predef.h for gcc >= 4.8
The gcc documentation says “On GNU/Linux, is pre-included.” See
https://gcc.gnu.org/gcc-4.8/porting_to.html
The preinclude is inhibited with –ffreestanding.
I’m not sure what other toolchains, if any, should support it. I put in some
changes for Solaris or MipsLinux but I didn’t debug them and not sure the
changes that i slapped in for MipsLinux and Solaris will, or should, work. I
could just omit those and let folks interested in the other toolchains to do
that, what’s your recommendation for supporting gcc on other platforms besides
Linux? I only debugged and verified on Linux. Also, what's the mechanism to
restrict the test case to run only on Linux?
Basically I fixed the failing test cases by adding either –ffreestanding or
–nostdinc which inhibits this behavior. Some of the tests didn't support
-ffreestanding but they did accept -nostdinc. Also, I have a few lit tests
failing in my sandbox, they'll need to be updated to accomodate the change in
the preprocessing. Anyway, hoping for some feedback on this patch.
in my sandbox, these tests are failing on Linux so currently it's not ready:
Failing Tests (7):
Clang Tools :: clang-tidy/llvm-include-order.cpp
Clang Tools :: clang-tidy/readability-implicit-bool-cast.cpp
Clang Tools :: pp-trace/pp-trace-pragma-general.cpp
Clang Tools :: pp-trace/pp-trace-pragma-opencl.cpp
Extra Tools Unit Tests :: clangd/ClangdTests/ClangdVFSTest.Parse
Extra Tools Unit Tests :: clangd/ClangdTests/ClangdVFSTest.ParseWithHeader
Extra Tools Unit Tests :: clangd/ClangdTests/ClangdVFSTest.Reparse
Repository:
rL LLVM
https://reviews.llvm.org/D34158
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Gnu.h
lib/Driver/ToolChains/Linux.cpp
lib/Driver/ToolChains/Linux.h
lib/Driver/ToolChains/MipsLinux.cpp
lib/Driver/ToolChains/MipsLinux.h
lib/Driver/ToolChains/Solaris.cpp
test/Driver/clang_cpp.c
test/Index/IBOutletCollection.m
test/Index/annotate-macro-args.m
test/Index/annotate-tokens-pp.c
test/Index/annotate-tokens.c
test/Index/c-index-getCursor-test.m
test/Index/get-cursor-macro-args.m
test/Index/get-cursor.cpp
unittests/Tooling/TestVisitor.h
Index: lib/Driver/ToolChains/MipsLinux.h
===
--- lib/Driver/ToolChains/MipsLinux.h
+++ lib/Driver/ToolChains/MipsLinux.h
@@ -28,6 +28,8 @@
void
AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const override;
CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList ) const override;
Index: lib/Driver/ToolChains/Solaris.cpp
===
--- lib/Driver/ToolChains/Solaris.cpp
+++ lib/Driver/ToolChains/Solaris.cpp
@@ -189,5 +189,6 @@
"." + Version.MinorStr + "/include/c++/" +
Version.Text + "/" +
GCCInstallation.getTriple().str());
+GCCInstallation.AddGnuIncludeArgs(DriverArgs, CC1Args);
}
}
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
void addLibStdCxxIncludePaths(
const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+ void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
void AddCudaIncludeArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/MipsLinux.cpp
===
--- lib/Driver/ToolChains/MipsLinux.cpp
+++ lib/Driver/ToolChains/MipsLinux.cpp
@@ -64,6 +64,12 @@
}
}
+void MipsLLVMToolChain::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const {
+ if (GCCInstallation.isValid())
+GCCInstallation.AddGnuIncludeArgs(DriverArgs, CC1Args);
+}
+
Tool *MipsLLVMToolChain::buildLinker() const {
return new tools::gnutools::Linker(*this);
}
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1771,6 +1771,16 @@
return false;
}
+void Generic_GCC::GCCInstallationDetector::AddGnuIncludeArgs(
+const ArgList , ArgStringList ) const {
+ if
