[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

2017-12-11 Thread Erich Keane via Phabricator via cfe-commits
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

2017-12-11 Thread Erich Keane via Phabricator via cfe-commits
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

2017-11-22 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-11-20 Thread Erich Keane via Phabricator via cfe-commits
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

2017-10-27 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-10-25 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-10-24 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-10-24 Thread James Y Knight via Phabricator via cfe-commits
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

2017-09-19 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-09-12 Thread Joerg Sonnenberger via cfe-commits
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

2017-09-12 Thread Blower, Melanie via cfe-commits


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

2017-09-12 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2017-09-12 Thread Melanie Blower via Phabricator via cfe-commits
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::pair Split = 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

2017-09-07 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-08-17 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-08-14 Thread Melanie Blower via Phabricator via cfe-commits
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::pair Split = 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

2017-08-11 Thread Melanie Blower via Phabricator via cfe-commits
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

2017-08-10 Thread James Y Knight via Phabricator via cfe-commits
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

2017-08-10 Thread Melanie Blower via Phabricator via cfe-commits
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::pair Split = 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

2017-08-10 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2017-08-10 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2017-08-09 Thread James Y Knight via Phabricator via cfe-commits
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

2017-08-09 Thread Hal Finkel via Phabricator via cfe-commits
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

2017-08-09 Thread Joerg Sonnenberger via Phabricator via cfe-commits
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

2017-08-08 Thread Fedor Sergeev via Phabricator via cfe-commits
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

2017-08-08 Thread James Y Knight via Phabricator via cfe-commits
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

2017-08-08 Thread Melanie Blower via Phabricator via cfe-commits
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::pair Split = 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

2017-08-08 Thread Blower, Melanie via cfe-commits
 
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

2017-08-07 Thread Fedor Sergeev via Phabricator via cfe-commits
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

2017-08-07 Thread Melanie Blower via Phabricator via cfe-commits
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::pair Split = StringRef(A->getValue()).split(';');
 
Index: