[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
This revision was automatically updated to reflect the committed changes. Closed by commit rL320391: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=123613=126386#toc Repository: rL LLVM https://reviews.llvm.org/D34158 Files: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Lex/PreprocessorOptions.h cfe/trunk/lib/Driver/Job.cpp cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h cfe/trunk/test/Driver/crash-report-header.h cfe/trunk/test/Driver/crash-report-spaces.c cfe/trunk/test/Driver/crash-report.c cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c cfe/trunk/test/Driver/stdc-predef.c cfe/trunk/test/Driver/stdc-predef.i cfe/trunk/test/Index/IBOutletCollection.m cfe/trunk/test/Index/annotate-macro-args.m cfe/trunk/test/Index/annotate-tokens-pp.c cfe/trunk/test/Index/annotate-tokens.c cfe/trunk/test/Index/c-index-getCursor-test.m cfe/trunk/test/Index/get-cursor-macro-args.m cfe/trunk/test/Index/get-cursor.cpp cfe/trunk/test/Preprocessor/ignore-pragmas.c cfe/trunk/unittests/Tooling/TestVisitor.h cfe/trunk/unittests/libclang/LibclangTest.cpp Index: cfe/trunk/unittests/Tooling/TestVisitor.h === --- cfe/trunk/unittests/Tooling/TestVisitor.h +++ cfe/trunk/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("-ffreestanding"); switch (L) { case Lang_C: Args.push_back("-x"); Index: cfe/trunk/unittests/libclang/LibclangTest.cpp === --- cfe/trunk/unittests/libclang/LibclangTest.cpp +++ cfe/trunk/unittests/libclang/LibclangTest.cpp @@ -434,8 +434,10 @@ "#ifdef KIWIS\n" "printf(\"mmm!!\");\n" "#endif"); + const char *Args[] = { "-ffreestanding" }; + int NumArgs = sizeof(Args) / sizeof(Args[0]); - ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0, + ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), Args, NumArgs, nullptr, 0, TUFlags); CXSourceRangeList *Ranges = clang_getAllSkippedRanges(ClangTU); Index: cfe/trunk/include/clang/Driver/CC1Options.td === --- cfe/trunk/include/clang/Driver/CC1Options.td +++ cfe/trunk/include/clang/Driver/CC1Options.td @@ -769,6 +769,8 @@ HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; +def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, + HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Index: cfe/trunk/include/clang/Lex/PreprocessorOptions.h === --- cfe/trunk/include/clang/Lex/PreprocessorOptions.h +++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h @@ -60,6 +60,9 @@ /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; + /// \brief System Headers that are pre-included if they exist. + std::vector FSystemIncludeIfExists; + /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation = false; @@ -183,6 +186,7 @@ DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); +FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Index: cfe/trunk/test/Index/annotate-tokens.c === --- cfe/trunk/test/Index/annotate-tokens.c +++ cfe/trunk/test/Index/annotate-tokens.c @@ -68,7 +68,7 @@ reg.field = 1; } -// RUN: c-index-test -test-annotate-tokens=%s:4:1:37:1 %s | FileCheck %s +// RUN: c-index-test -test-annotate-tokens=%s:4:1:37:1 -ffreestanding %s | FileCheck %s // CHECK: Identifier: "T" [4:3 - 4:4] TypeRef=T:1:13 // CHECK: Punctuation: "*" [4:4 - 4:5] VarDecl=t_ptr:4:6 (Definition) // CHECK: Identifier: "t_ptr" [4:6 - 4:11] VarDecl=t_ptr:4:6 (Definition) @@ -191,10 +191,10 @@ // CHECK: Punctuation: ")" [36:97 - 36:98] FunctionDecl=test:36:63 (unavailable) (always unavailable: "") // CHECK: Punctuation: ";" [36:98 - 36:99] -// RUN: c-index-test -test-annotate-tokens=%s:4:1:165:32 %s |
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
This revision was automatically updated to reflect the committed changes. Closed by commit rC320391: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=123613=126387#toc Repository: rC Clang https://reviews.llvm.org/D34158 Files: include/clang/Driver/CC1Options.td include/clang/Lex/PreprocessorOptions.h lib/Driver/Job.cpp lib/Driver/ToolChains/Linux.cpp lib/Driver/ToolChains/Linux.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c test/Driver/crash-report.c test/Driver/rewrite-map-in-diagnostics.c test/Driver/stdc-predef.c test/Driver/stdc-predef.i 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 unittests/libclang/LibclangTest.cpp Index: include/clang/Driver/CC1Options.td === --- include/clang/Driver/CC1Options.td +++ include/clang/Driver/CC1Options.td @@ -769,6 +769,8 @@ HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; +def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, + HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Index: include/clang/Lex/PreprocessorOptions.h === --- include/clang/Lex/PreprocessorOptions.h +++ include/clang/Lex/PreprocessorOptions.h @@ -60,6 +60,9 @@ /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; + /// \brief System Headers that are pre-included if they exist. + std::vector FSystemIncludeIfExists; + /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation = false; @@ -183,6 +186,7 @@ DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); +FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Index: test/Driver/crash-report-spaces.c === --- test/Driver/crash-report-spaces.c +++ test/Driver/crash-report-spaces.c @@ -1,7 +1,7 @@ // RUN: rm -rf "%t" // RUN: mkdir "%t" // RUN: cp "%s" "%t/crash report spaces.c" -// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s" +// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -ffreestanding -fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s" // RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC "%s" // RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH "%s" // REQUIRES: crash-recovery Index: test/Driver/rewrite-map-in-diagnostics.c === --- test/Driver/rewrite-map-in-diagnostics.c +++ test/Driver/rewrite-map-in-diagnostics.c @@ -1,7 +1,7 @@ // RUN: rm -rf "%t" // RUN: mkdir -p "%t" // RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTION=1 \ -// RUN: %clang -fsyntax-only -frewrite-map-file %p/Inputs/rewrite.map %s 2>&1 \ +// RUN: %clang -ffreestanding -fsyntax-only -frewrite-map-file %p/Inputs/rewrite.map %s 2>&1 \ // RUN: | FileCheck %s #pragma clang __debug parser_crash Index: test/Driver/stdc-predef.i === --- test/Driver/stdc-predef.i +++ test/Driver/stdc-predef.i @@ -0,0 +1,16 @@ +// The automatic preinclude of stdc-predef.h should not occur if +// the source filename indicates a preprocessed file. +// +// RUN: %clang %s -### -c 2>&1 \ +// RUN: --sysroot=%S/Inputs/stdc-predef \ +// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s + +int i; +// The automatic preinclude of stdc-predef.h should not occur if +// the source filename indicates a preprocessed file. +// +// RUN: %clang %s -### -c 2>&1 \ +// RUN: --sysroot=%S/Inputs/stdc-predef \ +// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s + +int i; Index: test/Driver/stdc-predef.c === --- test/Driver/stdc-predef.c +++ test/Driver/stdc-predef.c @@ -0,0 +1,25 @@ +// Test that clang preincludes stdc-predef.h, if the
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc reopened this revision. mibintc added a comment. Hi. I also posted this question on IRC @erichkeane pushed a fix for https://reviews.llvm.org/D34158 which on Linux does a preinclude of stdc-predef.h; on Monday. Later on Monday he pulled it out because it caused 3 test failures and word-of-mouth that it also caused chromium build to fail. I haven't been watching this IRC so I didn't catch the details about the chromium build fail. if you know, can you send me details about how/why chromium build fails? also, the 3 tests (2 lit tests, 1 google test) which failed on buildbot do not fail on the linux servers in-house at Intel. Any clue about why that would be? I have a patched compiler, and verify that the new functionality is present. I build check-clang and there are no fails. I looked with llvm-lit at each of the 2 failing tests and they don't fail, I checked the google test libclang "skip ranges" and it doesn't fail also maybe i should redirect this question elsewhere. don't hesitate to let me know also i have used the self-build for https://reviews.llvm.org/D34158 and likewise those 3 tests do not fail. i used "llvm-lit -a" on one of the failing tests test/Modules/crash-vfs-path-symlink-component.m to see why the test was failing. i was not able to run the commands directly from the linux shell the lit command that didn't work is not env FORCE_CLANG_DIAGNOSTICS_CRASH= ... it's the "not" which is not understood - does that ring a bell? it works OK within llvm-lit sptel9-ws/llorg1/builds/llorgefi2linux_debug/llvm/bin/count 1 ; the buildbot is showing 2 not 1 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: For Linux/gnu compatibility, preinclude if the file is available
This revision was automatically updated to reflect the committed changes. Closed by commit rL318669: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=120581=123613#toc Repository: rL LLVM https://reviews.llvm.org/D34158 Files: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Lex/PreprocessorOptions.h cfe/trunk/lib/Driver/Job.cpp cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/crash-report-header.h cfe/trunk/test/Driver/crash-report-spaces.c cfe/trunk/test/Driver/crash-report.c cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c cfe/trunk/test/Index/IBOutletCollection.m cfe/trunk/test/Index/annotate-macro-args.m cfe/trunk/test/Index/annotate-tokens-pp.c cfe/trunk/test/Index/annotate-tokens.c cfe/trunk/test/Index/c-index-getCursor-test.m cfe/trunk/test/Index/get-cursor-macro-args.m cfe/trunk/test/Index/get-cursor.cpp cfe/trunk/test/Preprocessor/ignore-pragmas.c cfe/trunk/unittests/Tooling/TestVisitor.h Index: cfe/trunk/unittests/Tooling/TestVisitor.h === --- cfe/trunk/unittests/Tooling/TestVisitor.h +++ cfe/trunk/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("-ffreestanding"); switch (L) { case Lang_C: Args.push_back("-x"); Index: cfe/trunk/include/clang/Driver/CC1Options.td === --- cfe/trunk/include/clang/Driver/CC1Options.td +++ cfe/trunk/include/clang/Driver/CC1Options.td @@ -769,6 +769,8 @@ HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; +def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, + HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Index: cfe/trunk/include/clang/Lex/PreprocessorOptions.h === --- cfe/trunk/include/clang/Lex/PreprocessorOptions.h +++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h @@ -60,6 +60,9 @@ /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; + /// \brief System Headers that are pre-included if they exist. + std::vector FSystemIncludeIfExists; + /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation; @@ -190,6 +193,7 @@ DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); +FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Index: cfe/trunk/test/Preprocessor/ignore-pragmas.c === --- cfe/trunk/test/Preprocessor/ignore-pragmas.c +++ cfe/trunk/test/Preprocessor/ignore-pragmas.c @@ -1,8 +1,8 @@ -// RUN: %clang_cc1 -E %s -Wall -verify -// RUN: %clang_cc1 -Eonly %s -Wall -verify -// RUN: %clang -M -Wall %s -Xclang -verify -// RUN: %clang -E -frewrite-includes %s -Wall -Xclang -verify -// RUN: %clang -E -dD -dM %s -Wall -Xclang -verify +// RUN: %clang_cc1 -E %s -Wall -ffreestanding -verify +// RUN: %clang_cc1 -Eonly %s -Wall -ffreestanding -verify +// RUN: %clang -M -Wall -ffreestanding %s -Xclang -verify +// RUN: %clang -E -frewrite-includes %s -Wall -ffreestanding -Xclang -verify +// RUN: %clang -E -dD -dM %s -Wall -ffreestanding -Xclang -verify // expected-no-diagnostics #pragma GCC visibility push (default) Index: cfe/trunk/test/Index/IBOutletCollection.m === --- cfe/trunk/test/Index/IBOutletCollection.m +++ cfe/trunk/test/Index/IBOutletCollection.m @@ -5,10 +5,10 @@ } @end -// RUN: c-index-test -cursor-at=%s:4:24 %s | FileCheck -check-prefix=CHECK-CURSOR %s +// RUN: c-index-test -cursor-at=%s:4:24 -ffreestanding %s | FileCheck -check-prefix=CHECK-CURSOR %s // CHECK-CURSOR: ObjCClassRef=Test:3:12 -// RUN: c-index-test -test-annotate-tokens=%s:4:1:5:1 %s | FileCheck -check-prefix=CHECK-TOK %s +// RUN: c-index-test -test-annotate-tokens=%s:4:1:5:1 -ffreestanding %s | FileCheck -check-prefix=CHECK-TOK %s // CHECK-TOK: Identifier: "IBOutletCollection" [4:3 - 4:21] macro expansion=IBOutletCollection:1:9 // FIXME: The following token should belong to the macro expansion cursor. // CHECK-TOK:
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc updated this revision to Diff 120581. mibintc added a comment. The patch to clang is the same as before. The modifications are to the test cases, I verified that all these test cases need modifications. I will also be updating the associated diff for clang/tools/extra, please review that as well. Without the test patches, these are the tests that fail (using check-all) . The failure mode is as described in a note I posted earlier this week, please let me know if you need further details. @jyknight Looking for your review and approval, thanks! Clang :: Driver/crash-report-header.h Clang :: Driver/crash-report-spaces.c Clang :: Driver/crash-report.c Clang :: Driver/rewrite-map-in-diagnostics.c Clang :: Driver/stdc-predef.c Clang :: Index/IBOutletCollection.m Clang :: Index/annotate-macro-args.m Clang :: Index/annotate-tokens-pp.c Clang :: Index/annotate-tokens.c Clang :: Index/c-index-getCursor-test.m Clang :: Index/get-cursor-macro-args.m Clang :: Index/get-cursor.cpp Clang :: Preprocessor/ignore-pragmas.c Clang Tools :: pp-trace/pp-trace-pragma-general.cpp Clang Tools :: pp-trace/pp-trace-pragma-opencl.cpp Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.BasicTest1 Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.BasicTest2 Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.BasicTest3 Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.IfBlock1 Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.IfBlock2 Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.IfBlock3 Clang-Unit :: Tooling/./ToolingTests/CommentHandlerTest.PPDirectives https://reviews.llvm.org/D34158 Files: include/clang/Driver/CC1Options.td include/clang/Lex/PreprocessorOptions.h lib/Driver/Job.cpp lib/Driver/ToolChains/Linux.cpp lib/Driver/ToolChains/Linux.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c test/Driver/crash-report.c test/Driver/rewrite-map-in-diagnostics.c test/Driver/stdc-predef.c test/Driver/stdc-predef.i 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: 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("-ffreestanding"); switch (L) { case Lang_C: Args.push_back("-x"); Index: test/Preprocessor/ignore-pragmas.c === --- test/Preprocessor/ignore-pragmas.c +++ test/Preprocessor/ignore-pragmas.c @@ -1,8 +1,8 @@ -// RUN: %clang_cc1 -E %s -Wall -verify -// RUN: %clang_cc1 -Eonly %s -Wall -verify -// RUN: %clang -M -Wall %s -Xclang -verify -// RUN: %clang -E -frewrite-includes %s -Wall -Xclang -verify -// RUN: %clang -E -dD -dM %s -Wall -Xclang -verify +// RUN: %clang_cc1 -E %s -Wall -ffreestanding -verify +// RUN: %clang_cc1 -Eonly %s -Wall -ffreestanding -verify +// RUN: %clang -M -Wall -ffreestanding %s -Xclang -verify +// RUN: %clang -E -frewrite-includes %s -Wall -ffreestanding -Xclang -verify +// RUN: %clang -E -dD -dM %s -Wall -ffreestanding -Xclang -verify // expected-no-diagnostics #pragma GCC visibility push (default) Index: test/Index/get-cursor.cpp === --- test/Index/get-cursor.cpp +++ test/Index/get-cursor.cpp @@ -152,71 +152,71 @@ void f_dynamic_noexcept() throw(int); void f_dynamic_noexcept_any() throw(...); -// RUN: c-index-test -cursor-at=%s:6:4 %s | FileCheck -check-prefix=CHECK-COMPLETION-1 %s +// RUN: c-index-test -cursor-at=%s:6:4 -ffreestanding %s | FileCheck -check-prefix=CHECK-COMPLETION-1 %s // CHECK-COMPLETION-1: CXXConstructor=X:6:3 // CHECK-COMPLETION-1-NEXT: Completion string: {TypedText X}{LeftParen (}{Placeholder int}{Comma , }{Placeholder int}{RightParen )} -// RUN: c-index-test -cursor-at=%s:31:16 %s | FileCheck -check-prefix=CHECK-COMPLETION-2 %s +// RUN: c-index-test -cursor-at=%s:31:16 -ffreestanding %s | FileCheck -check-prefix=CHECK-COMPLETION-2 %s // CHECK-COMPLETION-2: CXXMethod=getAnotherX:31:5 (Definition) // CHECK-COMPLETION-2-NEXT: Completion string: {ResultType X}{TypedText getAnotherX}{LeftParen (}{RightParen )} -// RUN: c-index-test -cursor-at=%s:12:20 %s | FileCheck -check-prefix=CHECK-VALUE-REF %s -// RUN: c-index-test -cursor-at=%s:13:21 %s | FileCheck -check-prefix=CHECK-VALUE-REF %s -// RUN:
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc added a comment. I pulled out all the test changes and the only one that is needed presently is Clang :: Driver/crash-report.c; huh, that's different than what I encountered earlier in the summer. Now I'll recheck those extra tests. 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc added a comment. In https://reviews.llvm.org/D34158#905596, @jyknight wrote: > I'd still _very_ much like a solution that is acceptable for all libc to use, > and have that on by default. > > However, I guess this is fine. > > Only concern I have is that it seems odd that so many test-cases needed to be > changed. What fails without those test changes? Those tests fail because they generate preprocessed output and then check carefully for precise results. Since the preprocessed output is different, the comparison fails. I "fixed" the tests by adding the freestanding option which inhibits the new behavior. I'll have to check out and rebuild everything so I can show you exactly the failure mode, I expect I can post details tomorrow. 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: For Linux/gnu compatibility, preinclude if the file is available
jyknight added a comment. I'd still _very_ much like a solution that is acceptable for all libc to use, and have that on by default. However, I guess this is fine. Only concern I have is that it seems odd that so many test-cases needed to be changed. What fails without those test changes? 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc added a comment. Hey @jyknight I heard from @erichkeane that you may want a couple more changes to this patch. Please let me know if you have some changes to recommend. @joerg thought this was OK for submission. Thanks --Melanie 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: For Linux/gnu compatibility, preinclude if the file is available
On Tue, Sep 12, 2017 at 08:12:26PM +, Blower, Melanie via cfe-commits wrote: > How is platform opt-in accomplished, is it part of the configure step? It is part of the Linux toolchain, other platforms interested in this or equivalent functionality would have to duplicate the hook. Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
> -Original Message- > From: Joerg Sonnenberger via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Tuesday, September 12, 2017 3:24 PM > To: Blower, Melanie; olivier...@gmail.com; > kalinichev.s...@gmail.com; kf...@kde.org; m...@milianw.de; Keane, Erich > ; mgo...@gentoo.org; fedor.v.serg...@gmail.com; > rich...@metafoo.co.uk; renato.go...@linaro.org > Cc: hfin...@anl.gov; jykni...@google.com; ibiryu...@google.com; cfe- > comm...@lists.llvm.org; kli...@google.com; simon.dar...@imgtec.com; > anastasia.stul...@arm.com; arichardson@gmail.com > Subject: [PATCH] D34158: For Linux/gnu compatibility, preinclude predef.h> if the file is available > > joerg added a comment. > > This version is fine with me. The only contentious part is whether it should > be > opt-in or opt-out for platforms, so getting this version in and revisiting > the issue > again later is OK from my perspective. > > > https://reviews.llvm.org/D34158 > > [Blower, Melanie] That's great news, thank you. Please note there are changes to some clang extra tests needed with this change. The review is here, https://reviews.llvm.org/D34624. You could patch the extra tests before accepting the patch for D34158 (the test changes aren't dependent on D34158). How is platform opt-in accomplished, is it part of the configure step? ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
joerg added a comment. This version is fine with me. The only contentious part is whether it should be opt-in or opt-out for platforms, so getting this version in and revisiting the issue again later is OK from my perspective. 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc updated this revision to Diff 114820. mibintc added a comment. I heard that @jyknight is still interested in this functionality, updating the patch to latest sources, to do this i needed to make a small change to a couple of driver tests. Other than that it's the same as previous submission. https://reviews.llvm.org/D34158 Files: include/clang/Driver/CC1Options.td include/clang/Lex/PreprocessorOptions.h lib/Driver/Job.cpp lib/Driver/ToolChains/Linux.cpp lib/Driver/ToolChains/Linux.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h test/Driver/clang_cpp.c test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c 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.c test/Driver/stdc-predef.i 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,17 @@ return ""; } +void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList , + llvm::opt::ArgStringList ) const { + if (!DriverArgs.hasArg(options::OPT_ffreestanding)) { +// For gcc compatibility, clang will preinclude +// -ffreestanding suppresses this behavior. +CC1Args.push_back("-fsystem-include-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/Driver/Job.cpp === --- lib/Driver/Job.cpp +++ lib/Driver/Job.cpp @@ -63,7 +63,7 @@ .Cases("-internal-externc-isystem", "-iprefix", true) .Cases("-iwithprefixbefore", "-isystem", "-iquote", true) .Cases("-isysroot", "-I", "-F", "-resource-dir", true) -.Cases("-iframework", "-include-pch", true) +.Cases("-iframework", "-include-pch", "-fsystem-include-if-exists", true) .Default(false); if (IsInclude) return HaveCrashVFS ? false : true; Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2563,6 +2563,10 @@ for (const Arg *A : Args.filtered(OPT_chain_include)) Opts.ChainedIncludes.emplace_back(A->getValue()); + // Add the ordered list of -fsystem-include-if-exists. + for (const Arg *A : Args.filtered(OPT_fsystem_include_if_exists)) +Opts.FSystemIncludeIfExists.emplace_back(A->getValue()); + for (const Arg *A : Args.filtered(OPT_remap_file)) { std::pairSplit = StringRef(A->getValue()).split(';'); Index: lib/Frontend/InitPreprocessor.cpp === --- lib/Frontend/InitPreprocessor.cpp +++ lib/Frontend/InitPreprocessor.cpp @@ -70,6 +70,15 @@ Builder.append(Twine("#include \"") + File + "\""); } +/// AddImplicitSystemIncludeIfExists - Add an implicit system \#include of the +/// specified file to the predefines buffer: precheck with __has_include. +static void AddImplicitSystemIncludeIfExists(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 + "\"");
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc added a comment. Another ping. Is it possible to approve this modification, or alternatively, let me know that it won't be approved indefinitely? I understand that it's a controversial change. Thanks! 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc added a comment. ping 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc updated this revision to Diff 111019. mibintc added a comment. @jyknight recommended adding the new option to skipArgs in Job.cpp. This revision makes that change and tests that in crash-report.c; look OK? https://reviews.llvm.org/D34158 Files: include/clang/Driver/CC1Options.td include/clang/Lex/PreprocessorOptions.h lib/Driver/Job.cpp lib/Driver/ToolChains/Linux.cpp lib/Driver/ToolChains/Linux.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h test/Driver/clang_cpp.c test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c 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.c test/Driver/stdc-predef.i 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,17 @@ return ""; } +void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList , + llvm::opt::ArgStringList ) const { + if (!DriverArgs.hasArg(options::OPT_ffreestanding)) { +// For gcc compatibility, clang will preinclude +// -ffreestanding suppresses this behavior. +CC1Args.push_back("-fsystem-include-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/Driver/Job.cpp === --- lib/Driver/Job.cpp +++ lib/Driver/Job.cpp @@ -63,7 +63,7 @@ .Cases("-internal-externc-isystem", "-iprefix", true) .Cases("-iwithprefixbefore", "-isystem", "-iquote", true) .Cases("-isysroot", "-I", "-F", "-resource-dir", true) -.Cases("-iframework", "-include-pch", true) +.Cases("-iframework", "-include-pch", "-fsystem-include-if-exists", true) .Default(false); if (IsInclude) return HaveCrashVFS ? false : true; Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2517,6 +2517,10 @@ for (const Arg *A : Args.filtered(OPT_chain_include)) Opts.ChainedIncludes.emplace_back(A->getValue()); + // Add the ordered list of -fsystem-include-if-exists. + for (const Arg *A : Args.filtered(OPT_fsystem_include_if_exists)) +Opts.FSystemIncludeIfExists.emplace_back(A->getValue()); + for (const Arg *A : Args.filtered(OPT_remap_file)) { std::pairSplit = StringRef(A->getValue()).split(';'); Index: lib/Frontend/InitPreprocessor.cpp === --- lib/Frontend/InitPreprocessor.cpp +++ lib/Frontend/InitPreprocessor.cpp @@ -70,6 +70,15 @@ Builder.append(Twine("#include \"") + File + "\""); } +/// AddImplicitSystemIncludeIfExists - Add an implicit system \#include of the +/// specified file to the predefines buffer: precheck with __has_include. +static void AddImplicitSystemIncludeIfExists(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. @@ -1105,6 +1114,13 @@ //
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc planned changes to this revision. mibintc added a comment. Need TO FIX: We should be stripping the new arg as well: add "-fsystem-include-if-exists" argument to the list of include things in the skipArgs() function in lib/Driver/Job.cpp 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: For Linux/gnu compatibility, preinclude if the file is available
jyknight added a comment. In https://reviews.llvm.org/D34158#838454, @mibintc wrote: > This patch is responding to @jyknight 's concern about preprocessed input. > The patch as it stands doesn't have this issue. I added 2 test cases, one > using option -x cpp-output, and another for a source file suffixed with .i > > Quoting James: "Firstly, let's consider a "clang foo.i" or "clang -x > cpp-output foo.c" compilation. In that case, it *clearly* should not be > including the predef file. I think the patch as it stands may not do this > properly. A test needs to be added for this to this patch, and perhaps the > behavior needs to be fixed as well." Thanks for the test. It wasn't obvious from the code, so I'm glad to hear it was already correct. :) Did you see the other suggestion I cleverly hid within a big block of commentary? "(TO FIX: We should be stripping the new arg as well: add "-fsystem-include-if-exists" argument to the list of include things in the skipArgs() function in lib/Driver/Job.cpp)" 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc updated this revision to Diff 110627. mibintc added a comment. This patch is responding to @jyknight 's concern about preprocessed input. The patch as it stands doesn't have this issue. I added 2 test cases, one using option -x cpp-output, and another for a source file suffixed with .i Quoting James: "Firstly, let's consider a "clang foo.i" or "clang -x cpp-output foo.c" compilation. In that case, it *clearly* should not be including the predef file. I think the patch as it stands may not do this properly. A test needs to be added for this to this patch, and perhaps the behavior needs to be fixed as well." 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/usr/include/stdc-predef.h test/Driver/clang_cpp.c test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c 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.c test/Driver/stdc-predef.i 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,17 @@ return ""; } +void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList , + llvm::opt::ArgStringList ) const { + if (!DriverArgs.hasArg(options::OPT_ffreestanding)) { +// For gcc compatibility, clang will preinclude +// -ffreestanding suppresses this behavior. +CC1Args.push_back("-fsystem-include-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 @@ -2517,6 +2517,10 @@ for (const Arg *A : Args.filtered(OPT_chain_include)) Opts.ChainedIncludes.emplace_back(A->getValue()); + // Add the ordered list of -fsystem-include-if-exists. + for (const Arg *A : Args.filtered(OPT_fsystem_include_if_exists)) +Opts.FSystemIncludeIfExists.emplace_back(A->getValue()); + for (const Arg *A : Args.filtered(OPT_remap_file)) { std::pairSplit = StringRef(A->getValue()).split(';'); Index: lib/Frontend/InitPreprocessor.cpp === --- lib/Frontend/InitPreprocessor.cpp +++ lib/Frontend/InitPreprocessor.cpp @@ -70,6 +70,15 @@ Builder.append(Twine("#include \"") + File + "\""); } +/// AddImplicitSystemIncludeIfExists - Add an implicit system \#include of the +/// specified file to the predefines buffer: precheck with __has_include. +static void AddImplicitSystemIncludeIfExists(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. @@ -1104,6 +1113,13 @@ // Exit the command line and go back to (2 is LC_LEAVE). if (!PP.getLangOpts().AsmPreprocessor) Builder.append("# 1 \"\" 2"); + + // Process
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
joerg added a comment. In https://reviews.llvm.org/D34158#837444, @jyknight wrote: > Now, the "/tmp/foo-XXX.sh" also has a line labeled "Driver args: " with the > original command-line on it. If I understand correctly, you then like to take > this > simpler Driver command-line, and edit it manually: add -save-temps, and > change the input filename to the "/tmp/foo-XXX.c" file, and run that, instead > of actually invoking the reproducer foo-XXX.sh. It's not so much that it is simpler, but the only reasonable easy way for forcing preprocessor output to written. That one is much nicer for passing to creduce for example (after stripping the remaining # lines). > Since stdc-predef.h is included automatically, it will now be present twice > -- first, it will read the one from your system's /usr/include, and then the > copy inlined into the > /tmp/foo-XXX.c file. That's not what you desired. You wanted nothing from > your /usr/include to be used. Correct. Worse, the include may file depending on the host system and give different results from the original build, resulting in annoying debugging seasons. > The fix for the end-user here is easy: you can add -nostdinc which will > suppress all the default include paths, and thus it will not find this predef > file from your system include dir. Yeah, except you have to remember that when it was never needed before. > I'll note that you'd also have had an issue if the original driver > command-line had a "-include" option in it, which you would have needed to > edit out > manually as well. (But I understand that is less common.) Yes, -include is rarely used in the wild. > Have I correctly described the situation? I guess my feeling here is that > this is somewhat of an edge-case, and that the workaround (-nostdinc) is a > sufficient fix. > (3) It seems to me that the GNU userland (and maybe Windows) is the exception to a well integrated tool chain. Most other platforms have a single canonical libc, libm and libpthread implementation and can as such directly define all the relevant macros directly in the driver. >>> >>> I don't think this is accurate. There's many platforms out there, and for >>> almost none of them do we have exact knowledge of the features of the libc >>> encoded >>> into the compiler. I'd note that not only do you need this for every (OS, >>> libc) combination, you'd need it for every (OS, libc-VERSION) combination. >> >> Not really. The feature set is rarely changing and generally will have only >> a cut-off version. > > Yes, but this is information that the compiler has no real need to know, and > that for many platforms would be difficult and problematic to coordinate. That's kind of my point. There is little coordination involved for almost all platforms since they provide a consistent user environment as a single package. Pushing the changes into Clang (or GCC for that matter) is trivial if you do not have to deal with multiple different versions of libc, the kernel and whatever in a mix-and-match scenario. 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: For Linux/gnu compatibility, preinclude if the file is available
joerg added a comment. In https://reviews.llvm.org/D34158#837281, @hfinkel wrote: > In https://reviews.llvm.org/D34158#837130, @joerg wrote: > > > In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > > > > > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > > > > > (2) It adds magic behavior that can make debugging more difficult. > > > > Partially preprocessed sources for example could be compiled with plain > > > > -c before, now they need a different command line. > > > > > > > > > If this is a problem, making it be Linux-only does _nothing_ to solve it. > > > But I don't actually see how this is a substantively new problem? > > > Compiling with plain -c before > > > would get #defines for those predefined macros that the compiler sets, > > > even though you may not have wanted those. Is this fundamentally > > > different? > > > > > > It makes it a linux-only problem. As such, it is something *I* only care > > about secondary. A typical use case I care about a lot is pulling the crash > > report sources from my (NetBSD) build machine, > > extracting the original command line to rerun the normal compilation with > > -save-temps. I don't necessarily have the (same) system headers on the > > machine I use for debugging and that's exactly > > the kind of use case this change breaks. All other predefined macros are > > driven by the target triple and remain stable. > > > Don't you use preprocessed source files from a crash? The crash rewrite tool creates semi-preprocessed output. It resolves includes along all code branches, but keeps macros and CPP conditionals alone. 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: For Linux/gnu compatibility, preinclude if the file is available
jyknight added a comment. Just to restate: the ideal outcome of this discussion for me would be to resolve things such that _ALL_ libc implementations will feel comfortable using this technique to provide the C11-required predefined macros. I'd love for linux, freebsd, macos, solaris, etc etc libc to all conform to the C standard in this regards, and do so in a common way, without the need to encode information about each libc version into the compiler. I _really_ don't think that scales well. So I take your comments from FreeBSD's point of view seriously, and would very much like to understand and hopefully resolve them. In https://reviews.llvm.org/D34158#837130, @joerg wrote: > In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > > > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > > > (2) It adds magic behavior that can make debugging more difficult. > > > Partially preprocessed sources for example could be compiled with plain > > > -c before, now they need a different command line. > > > > > > If this is a problem, making it be Linux-only does _nothing_ to solve it. > > But I don't actually see how this is a substantively new problem? Compiling > > with plain -c before > > would get #defines for those predefined macros that the compiler sets, > > even though you may not have wanted those. Is this fundamentally different? > > > It makes it a linux-only problem. As such, it is something *I* only care > about secondary. A typical use case I care about a lot is pulling the crash > report sources from my (NetBSD) build machine, > extracting the original command line to rerun the normal compilation with > -save-temps. I don't necessarily have the (same) system headers on the > machine I use for debugging and that's exactly > the kind of use case this change breaks. All other predefined macros are > driven by the target triple and remain stable. "it's Linux only so I don't care if it's broken." is still not very helpful. :) But I do think understand what you're saying now, so thanks for the elaboration. Firstly, let's consider a "clang foo.i" or "clang -x cpp-output foo.c" compilation. In that case, it *clearly* should not be including the predef file. I think the patch as it stands may not do this properly. A test needs to be added for this to this patch, and perhaps the behavior needs to be fixed as well. (Sidenote: clang doesn't support preprocessed input properly, but that's another bug, and we certainly ought not make it worse. Check out e.g. "int main() { return __GNUC__; }". it should report that __GNUC__ is undeclared, but instead compiles a program that returns 4.) But, that's not the case you're talking about above -- you're not talking about compiling preprocessed output, you're talking about taking output that comes from -frewrite-includes. Let me recap the scenario: 1. Start with a source file foo.c, with this content: #include #pragma clang __debug parser_crash 2. Run "clang foo.c". It crashes, and dumps a /tmp/foo-XXX.c and a /tmp/foo-XXX.sh script. The .c file is generated via -frewrite-includes, so it's _not_ already preprocessed, it simply has all includes pulled into a single file. It also _doesn't_ insert the compiler-predefined macros at the top, but it _will_ include the content of this stdc-predef.h file. OK, so then... The generated script invokes a -cc1 command line, with all the include arguments stripped out of the command. (TO FIX: We should be stripping the new arg as well: add "-fsystem-include-if-exists" argument to the list of include things in the skipArgs() function in lib/Driver/Job.cpp). Even without that change, it's actually already fine, as there is no include path specified in which to find the file -- but it's cleaner to strip it, so let's do that. The reproducer script will thus run correctly, and not include the file. Now, the "/tmp/foo-XXX.sh" also has a line labeled "Driver args: " with the original command-line on it. If I understand correctly, you then like to take this simpler Driver command-line, and edit it manually: add -save-temps, and change the input filename to the "/tmp/foo-XXX.c" file, and run that, instead of actually invoking the reproducer foo-XXX.sh. Since stdc-predef.h is included automatically, it will now be present twice -- first, it will read the one from your system's /usr/include, and then the copy inlined into the /tmp/foo-XXX.c file. That's not what you desired. You wanted nothing from your /usr/include to be used. The fix for the end-user here is easy: you can add -nostdinc which will suppress all the default include paths, and thus it will not find this predef file from your system include dir. I'll note that you'd also have had an issue if the original driver command-line had a "-include" option in it, which you would have needed to edit out manually as well. (But I understand that is less common.) Have I correctly described the situation? I
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
hfinkel added a comment. In https://reviews.llvm.org/D34158#837130, @joerg wrote: > In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > > > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > > > (2) It adds magic behavior that can make debugging more difficult. > > > Partially preprocessed sources for example could be compiled with plain > > > -c before, now they need a different command line. > > > > > > If this is a problem, making it be Linux-only does _nothing_ to solve it. > > But I don't actually see how this is a substantively new problem? Compiling > > with plain -c before > > would get #defines for those predefined macros that the compiler sets, > > even though you may not have wanted those. Is this fundamentally different? > > > It makes it a linux-only problem. As such, it is something *I* only care > about secondary. A typical use case I care about a lot is pulling the crash > report sources from my (NetBSD) build machine, > extracting the original command line to rerun the normal compilation with > -save-temps. I don't necessarily have the (same) system headers on the > machine I use for debugging and that's exactly > the kind of use case this change breaks. All other predefined macros are > driven by the target triple and remain stable. Don't you use preprocessed source files from a crash? 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: For Linux/gnu compatibility, preinclude if the file is available
joerg added a comment. In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > (2) It adds magic behavior that can make debugging more difficult. > > Partially preprocessed sources for example could be compiled with plain -c > > before, now they need a different command line. > > > If this is a problem, making it be Linux-only does _nothing_ to solve it. But > I don't actually see how this is a substantively new problem? Compiling with > plain -c before > would get #defines for those predefined macros that the compiler sets, even > though you may not have wanted those. Is this fundamentally different? It makes it a linux-only problem. As such, it is something *I* only care about secondary. A typical use case I care about a lot is pulling the crash report sources from my (NetBSD) build machine, extracting the original command line to rerun the normal compilation with -save-temps. I don't necessarily have the (same) system headers on the machine I use for debugging and that's exactly the kind of use case this change breaks. All other predefined macros are driven by the target triple and remain stable. >> (3) It seems to me that the GNU userland (and maybe Windows) is the >> exception to a well integrated tool chain. Most other platforms have a >> single canonical >> libc, libm and libpthread implementation and can as such directly define >> all the relevant macros directly in the driver. > > I don't think this is accurate. There's many platforms out there, and for > almost none of them do we have exact knowledge of the features of the libc > encoded > into the compiler. I'd note that not only do you need this for every (OS, > libc) combination, you'd need it for every (OS, libc-VERSION) combination. Not really. The feature set is rarely changing and generally will have only a cut-off version. >> Given that many of the macros involved are already reflected by the compiler >> behavior anyway, they can't be decoupled. I.e. the questionable >> concept of locale-independent wchar_t is already hard-coded in the front >> end as soon as any long character literals are used. > > AFAICT, this example is not actually the case. The frontend only needs to > know *a* valid encoding for wide-character literals in some > implementation-defined locale (for example, it might always emit them as > unicode codepoints, as clang does). Standard says: > "the array elements [...] are initialized with the sequence of wide > characters corresponding to the multibyte character sequence, as > defined by the mbstowcs function with an implementation defined current > locale." I know what the standard says. It doesn't make much sense if you do not have a fixed wchar_t encoding. > On the other hand, I believe the intent of this macro is to guarantee that > _no matter what_ the locale is, > that a U+0100 character (say, translated with mbrtowc from the locale > encoding) gets represented as 0x100. Yes, so it is essentially "we have screwed up by creating a language mechanism that adds a major constraint, so let's go all the way". > Thus, it's "fine" for the frontend to always emit wchar_t literals as > unicode, yet, the libc may sometimes use an arbitrary different > internal encoding, depending on the locale used at runtime. (Obviously using > wide character literals with such a libc would be a poor > user experience, and such a libc probably ought to reconsider its choices, > but that's a different discussion.) One of the biggest opponents of that was Itojun. It's not a decision that should be made here as it is only indirectly related to this discussion. >> As such, please move the command line additions back into the >> target-specific files for targets that actually want to get this behavior. > > Without even a suggestion of a better solution to use for other targets, I > find it is to be a real shame to push for this to be linux-only, > and leave everyone else hanging. I find that that _most_ of these defines > are effectively library decisions. I further would claim that these > are likely to vary over the lifetime of even a single libc, and that as such > we would be better served by allowing the libc to set them directly, rather > than encoding it into the compiler. > > TTBOMK, no targets except linux/glibc/gcc actually comply with this part of > the C99/C11 standard today, and so this feature would be useful to have > available across all targets. > > (I very much dislike that the C standard has started adding all these new > predefined macros, instead of exposing them from a header, but there's not > much to be done about that...) Exactly. It's not like this is a lot of target logic. It should be a single call for targets that want to get this functionality. But that's my point -- it should be opt-in, not opt-out. https://reviews.llvm.org/D34158
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
fedor.sergeev added a comment. In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > I had a long discussion with James about this on IRC without reaching a > > clear consensus. I consider forcing this behavior on all targets to be a > > major bug. It should be opt-in and opt-in only: > > > > (1) The header name is not mandated by any standard. It is not in any > > namespace generally accepted as implementation-owned. > > > This is a point. I didn't think it was a big deal, but if you want to argue a > different name should be used, that's a reasonable argument. > If we can get some agreement amongst other libc vendors to use some more > agreeable alternative name, and keep a fallback on linux-only for the > "stdc-predef.h" name, I'd consider that as a great success. Perhaps not a big deal yet, but as I have recently described stdc-predef.h idea to Oracle Solaris libc/headers/compilers folks, they generally welcomed the idea.. >> (3) ...Most other platforms have a single canonical libc, libm and >> libpthread implementation and can as such directly define all the relevant >> macros directly in the driver. > > I don't think this is accurate. There's many platforms out there, and for > almost none of them do we have exact knowledge of the features of the libc > encoded into the compiler. Solaris is a direct example of that... 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: For Linux/gnu compatibility, preinclude if the file is available
jyknight added a comment. In https://reviews.llvm.org/D34158#827178, @joerg wrote: > I had a long discussion with James about this on IRC without reaching a clear > consensus. I consider forcing this behavior on all targets to be a major bug. > It should be opt-in and opt-in only: > > (1) The header name is not mandated by any standard. It is not in any > namespace generally accepted as implementation-owned. This is a point. I didn't think it was a big deal, but if you want to argue a different name should be used, that's a reasonable argument. If we can get some agreement amongst other libc vendors to use some more agreeable alternative name, and keep a fallback on linux-only for the "stdc-predef.h" name, I'd consider that as a great success. > (2) It adds magic behavior that can make debugging more difficult. Partially > preprocessed sources for example could be compiled with plain -c before, now > they need a different command line. If this is a problem, making it be Linux-only does _nothing_ to solve it. But I don't actually see how this is a substantively new problem? Compiling with plain -c before would get #defines for those predefined macros that the compiler sets, even though you may not have wanted those. Is this fundamentally different? > (3) It seems to me that the GNU userland (and maybe Windows) is the exception > to a well integrated tool chain. Most other platforms have a single canonical > libc, libm and libpthread implementation and can as such directly define all > the relevant macros directly in the driver. I don't think this is accurate. There's many platforms out there, and for almost none of them do we have exact knowledge of the features of the libc encoded into the compiler. I'd note that not only do you need this for every (OS, libc) combination, you'd need it for every (OS, libc-VERSION) combination. > Given that many of the macros involved are already reflected by the compiler > behavior anyway, they can't be decoupled. I.e. the questionable concept of > locale-independent wchar_t is already hard-coded in the front end as soon as > any long character literals are used. AFAICT, this example is not actually the case. The frontend only needs to know *a* valid encoding for wide-character literals in some implementation-defined locale (for example, it might always emit them as unicode codepoints, as clang does). Standard says: "the array elements [...] are initialized with the sequence of wide characters corresponding to the multibyte character sequence, as defined by the mbstowcs function with an implementation defined current locale." On the other hand, I believe the intent of this macro is to guarantee that _no matter what_ the locale is, that a U+0100 character (say, translated with mbrtowc from the locale encoding) gets represented as 0x100. Thus, it's "fine" for the frontend to always emit wchar_t literals as unicode, yet, the libc may sometimes use an arbitrary different internal encoding, depending on the locale used at runtime. (Obviously using wide character literals with such a libc would be a poor user experience, and such a libc probably ought to reconsider its choices, but that's a different discussion.) > As such, please move the command line additions back into the target-specific > files for targets that actually want to get this behavior. Without even a suggestion of a better solution to use for other targets, I find it is to be a real shame to push for this to be linux-only, and leave everyone else hanging. I find that that _most_ of these defines are effectively library decisions. I further would claim that these are likely to vary over the lifetime of even a single libc, and that as such we would be better served by allowing the libc to set them directly, rather than encoding it into the compiler. TTBOMK, no targets except linux/glibc/gcc actually comply with this part of the C99/C11 standard today, and so this feature would be useful to have available across all targets. (I very much dislike that the C standard has started adding all these new predefined macros, instead of exposing them from a header, but there's not much to be done about that...) Going through the various macros: `__STDC_ISO_10646__`: As discussed above, this is effectively entirely up to the libc. The compiler only need support one possible encoding for wchar_t, and clang always chooses unicode. But it can't define this because the libc might use others as well. `__STDC_MB_MIGHT_NEQ_WC__`: As with `__STDC_ISO_10646__`, this is up to the libc not the compiler. (At least, I think so... I note that clang currently sets this for freebsd, with a FIXME next to it saying it's only intended to apply to wide character literals. I don't see that the standard says that, however, so, IMO, having it set for freebsd was and is correct). `__STDC_UTF16__`, `__STDC_UTF32__`: Again, analogous to `__STDC_ISO_10646__`, except dealing with
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
mibintc updated this revision to Diff 110193. mibintc added a comment. Responding to @fedor.sergeev 's comment. This is an updated patch which pulls out the "isValid" check on GCCInstallation. Also I'm putting back the dummy sysroot tree which contains stdc-predef.h and adding a new test run to confirm that the new option fsystem-include-if-exists is actually working. 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/usr/include/stdc-predef.h test/Driver/clang_cpp.c test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c 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.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,17 @@ return ""; } +void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList , + llvm::opt::ArgStringList ) const { + if (!DriverArgs.hasArg(options::OPT_ffreestanding)) { +// For gcc compatibility, clang will preinclude +// -ffreestanding suppresses this behavior. +CC1Args.push_back("-fsystem-include-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 @@ -2503,6 +2503,10 @@ for (const Arg *A : Args.filtered(OPT_chain_include)) Opts.ChainedIncludes.emplace_back(A->getValue()); + // Add the ordered list of -fsystem-include-if-exists. + for (const Arg *A : Args.filtered(OPT_fsystem_include_if_exists)) +Opts.FSystemIncludeIfExists.emplace_back(A->getValue()); + for (const Arg *A : Args.filtered(OPT_remap_file)) { std::pairSplit = StringRef(A->getValue()).split(';'); Index: lib/Frontend/InitPreprocessor.cpp === --- lib/Frontend/InitPreprocessor.cpp +++ lib/Frontend/InitPreprocessor.cpp @@ -70,6 +70,15 @@ Builder.append(Twine("#include \"") + File + "\""); } +/// AddImplicitSystemIncludeIfExists - Add an implicit system \#include of the +/// specified file to the predefines buffer: precheck with __has_include. +static void AddImplicitSystemIncludeIfExists(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. @@ -1104,6 +1113,13 @@ // Exit the command line and go back to (2 is LC_LEAVE). if (!PP.getLangOpts().AsmPreprocessor) Builder.append("# 1 \"\" 2"); + + // Process -fsystem-include-if-exists directives. + for (unsigned i = 0, + e = InitOpts.FSystemIncludeIfExists.size(); i != e; ++i) { +const std::string = InitOpts.FSystemIncludeIfExists[i]; +AddImplicitSystemIncludeIfExists(Builder, Path); + } // If -imacros are specified,
RE: [PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
fedor.sergeev added a comment. In https://reviews.llvm.org/D34158#834298, @mibintc wrote: > In fact I did have trouble writing the new test case to pass with the > gnu/Linux toolchain. In the file lib/Driver/ToolChains/Linux.cpp function > AddGnuIncludeArgs checks if GCCInstallation.isValid(). You should not be doing stdc-predef.h under GCCInstallation.isValid(). You are handling interaction with libc, so it has nothing to do with the presence or absence of gcc toolchain. >> Thanks, Fedor. I'm uploading another patch which pulls out the "isValid" >> check. Also I'm putting back the dummy sysroot tree which contains >> stdc-predef.h and adding a new test run to confirm that the new option >> fsystem-include-if-exists is actually working. 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: For Linux/gnu compatibility, preinclude if the file is available
fedor.sergeev added a comment. In https://reviews.llvm.org/D34158#834298, @mibintc wrote: > In fact I did have trouble writing the new test case to pass with the > gnu/Linux toolchain. In the file lib/Driver/ToolChains/Linux.cpp function > AddGnuIncludeArgs checks if GCCInstallation.isValid(). You should not be doing stdc-predef.h under GCCInstallation.isValid(). You are handling interaction with libc, so it has nothing to do with the presence or absence of gcc toolchain. 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: For Linux/gnu compatibility, preinclude if the file is available
mibintc updated this revision to Diff 110055. mibintc added a comment. In the last review, it was deemed less controversial if I move these updates back into the gnu/Linux tool chain. This revision is responding to that feedback. I also simplified the test case. I tested on Linux with check-all and check-clang and found no unexpected failures. The other test case modifications are to fix tests which are broken with this change due to unexpected changes to the preprocessing. I changed all these to pass -ffreestanding which inhibits the new behavior. In fact I did have trouble writing the new test case to pass with the gnu/Linux toolchain. In the file lib/Driver/ToolChains/Linux.cpp function AddGnuIncludeArgs checks if GCCInstallation.isValid(). I don't know how to set up a Driver/Inputs sysroot tree so that the isValid check passes. (Does anyone know?) I experimented with various existing trees and had success with basic_cross_linux_tree, so I used that one in the test case. However basic_cross_linux_tree doesn't contain stdc-predef.h. Would it be OK to add an include file to that tree? (I don't know if it's acceptable to add a new sysroot tree: it is a lot of new files and directories. On the other hand, I don't know if it's OK to co-opt a tree which was added by a different test case) Perhaps you want me to another test case which verifies that fsystem-include-if-exists works generally. Currently I've added an affirmative test to see that -fsystem-include-if-exists is on the command line, and a test that -ffreestanding inhibits, and a negative test that when given a sysroot without the file, that the preprocessed output shows no indication of stdc-predef.h. 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/clang_cpp.c test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c 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.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,19 @@ return ""; } +void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList , + llvm::opt::ArgStringList ) const { + if (GCCInstallation.isValid()) { +if (!DriverArgs.hasArg(options::OPT_ffreestanding)) { + // For gcc compatibility, clang will preinclude + // -ffreestanding suppresses this behavior. + CC1Args.push_back("-fsystem-include-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 @@ -2503,6 +2503,10 @@ for (const Arg *A : Args.filtered(OPT_chain_include)) Opts.ChainedIncludes.emplace_back(A->getValue()); + // Add the ordered list of -fsystem-include-if-exists. + for (const Arg *A : Args.filtered(OPT_fsystem_include_if_exists)) +Opts.FSystemIncludeIfExists.emplace_back(A->getValue()); + for (const Arg *A : Args.filtered(OPT_remap_file)) { std::pairSplit = StringRef(A->getValue()).split(';'); Index: