Lekensteyn created this revision.
Herald added a subscriber: klimek.

GCC defaults to expanding the dependencies in depfiles, resolving
components like ".." and symlinks. Mimic this feature to ensure that the
Ninja build tool detects the correct dependencies when a symlink changes
directory levels, see https://github.com/ninja-build/ninja/issues/1330

An option to disable this feature is added in case "these changed header
paths may conflict with some compilation environments", see
https://gcc.gnu.org/ml/gcc-patches/2012-09/msg00287.html

Note that under GCC, this option would also affect the paths shown in
the preprocessed output (-E), but that is not implemented here since I
am not sure if it is useful or breaks something else.


https://reviews.llvm.org/D37954

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/DependencyOutputOptions.h
  lib/Driver/Job.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/DependencyFile.cpp
  lib/Tooling/ArgumentsAdjusters.cpp
  test/Preprocessor/dependencies-realpath.c

Index: test/Preprocessor/dependencies-realpath.c
===================================================================
--- /dev/null
+++ test/Preprocessor/dependencies-realpath.c
@@ -0,0 +1,32 @@
+// RUN: mkdir -p %T/sub/dir
+// RUN: echo > %T/sub/empty.h
+
+// Test that absolute system header paths are expanded in -MD
+//
+// RUN: %clang -fsyntax-only -MD -MF %t.d -MT foo %s -isystem %T/sub/dir/..
+// RUN: FileCheck -check-prefix=TEST1 %s < %t.d
+// TEST1: foo:
+// TEST1: sub{{/|\\}}empty.h
+
+// Test that relative system header paths are not expanded in -MD
+//
+// RUN: cd %T && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -isystem sub/dir/..
+// RUN: FileCheck -check-prefix=TEST2 %s < %t.d
+// TEST2: foo:
+// TEST2: sub/dir/..{{/|\\}}empty.h
+
+// Test that absolute user header paths are not expanded in -MD
+//
+// RUN: cd %T && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %T/sub/dir/..
+// RUN: FileCheck -check-prefix=TEST3 %s < %t.d
+// TEST3: foo:
+// TEST3: sub/dir/..{{/|\\}}empty.h
+
+// Test that absolute system header paths are not expanded in -MD with -fno-canonical-system-headers
+//
+// RUN: cd %T && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %T/sub/dir/.. -fno-canonical-system-headers
+// RUN: FileCheck -check-prefix=TEST4 %s < %t.d
+// TEST4: foo:
+// TEST4: sub/dir/..{{/|\\}}empty.h
+
+#include <empty.h>
Index: lib/Tooling/ArgumentsAdjusters.cpp
===================================================================
--- lib/Tooling/ArgumentsAdjusters.cpp
+++ lib/Tooling/ArgumentsAdjusters.cpp
@@ -58,7 +58,8 @@
       StringRef Arg = Args[i];
       // All dependency-file options begin with -M. These include -MM,
       // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-      if (!Arg.startswith("-M"))
+      // The exception is -fno-canonical-system-headers.
+      if (!Arg.startswith("-M") && !Arg.equals("-fno-canonical-system-headers"))
         AdjustedArgs.push_back(Args[i]);
 
       if ((Arg == "-MF") || (Arg == "-MT") || (Arg == "-MQ") ||
Index: lib/Frontend/DependencyFile.cpp
===================================================================
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -161,6 +161,7 @@
   bool AddMissingHeaderDeps;
   bool SeenMissingHeader;
   bool IncludeModuleFiles;
+  bool CanonicalSystemHeaders;
   DependencyOutputFormat OutputFormat;
 
 private:
@@ -176,6 +177,7 @@
       AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
       SeenMissingHeader(false),
       IncludeModuleFiles(Opts.IncludeModuleFiles),
+      CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
       OutputFormat(Opts.OutputFormat) {
     for (const auto &ExtraDep : Opts.ExtraDeps) {
       AddFilename(ExtraDep);
@@ -288,6 +290,16 @@
   if (!FileMatchesDepCriteria(Filename.data(), FileType))
     return;
 
+  // Canonicalize absolute system header paths like GCC does (unless
+  // -fno-canonical-system-headers is given).
+  if (CanonicalSystemHeaders && isSystem(FileType) &&
+      llvm::sys::path::is_absolute(Filename)) {
+    StringRef RealPath = FE->tryGetRealPathName();
+    if (!RealPath.empty()) {
+      Filename = RealPath;
+    }
+  }
+
   AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
 }
 
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -998,6 +998,7 @@
   Opts.Targets = Args.getAllArgValues(OPT_MT);
   Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);
   Opts.IncludeModuleFiles = Args.hasArg(OPT_module_file_deps);
+  Opts.CanonicalSystemHeaders = !Args.hasArg(OPT_fno_canonical_system_headers);
   Opts.UsePhonyTargets = Args.hasArg(OPT_MP);
   Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -979,6 +979,10 @@
   Args.AddLastArg(CmdArgs, options::OPT_C);
   Args.AddLastArg(CmdArgs, options::OPT_CC);
 
+  if (Args.hasArg(options::OPT_fno_canonical_system_headers)) {
+    CmdArgs.push_back("-fno-canonical-system-headers");
+  }
+
   // Handle dependency file generation.
   if ((A = Args.getLastArg(options::OPT_M, options::OPT_MM)) ||
       (A = Args.getLastArg(options::OPT_MD)) ||
Index: lib/Driver/Job.cpp
===================================================================
--- lib/Driver/Job.cpp
+++ lib/Driver/Job.cpp
@@ -74,7 +74,7 @@
   // These flags are all of the form -Flag and have no second argument.
   ShouldSkip = llvm::StringSwitch<bool>(Flag)
     .Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
-    .Case("-MMD", true)
+    .Cases("-MMD", "-fno-canonical-system-headers", true)
     .Default(false);
 
   // Match found.
Index: include/clang/Frontend/DependencyOutputOptions.h
===================================================================
--- include/clang/Frontend/DependencyOutputOptions.h
+++ include/clang/Frontend/DependencyOutputOptions.h
@@ -30,6 +30,8 @@
   unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency list
   unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info.
   unsigned IncludeModuleFiles : 1; ///< Include module file dependencies.
+  unsigned CanonicalSystemHeaders : 1; ///< Use the real path for system header
+                                       /// dependencies.
 
   /// The format for the dependency file.
   DependencyOutputFormat OutputFormat;
@@ -67,6 +69,7 @@
     AddMissingHeaderDeps = 0;
     PrintShowIncludes = 0;
     IncludeModuleFiles = 0;
+    CanonicalSystemHeaders = 1;
     OutputFormat = DependencyOutputFormat::Make;
   }
 };
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -380,6 +380,9 @@
     HelpText<"Specify name of main file output in depfile">;
 def MV : Flag<["-"], "MV">, Group<M_Group>, Flags<[CC1Option]>,
     HelpText<"Use NMake/Jom format for the depfile">;
+def fno_canonical_system_headers : Flag<["-"], "fno-canonical-system-headers">,
+    Group<M_Group>, Flags<[CC1Option]>,
+    HelpText<"Do not shorten system header paths in depfiles">;
 def Mach : Flag<["-"], "Mach">, Group<Link_Group>;
 def O0 : Flag<["-"], "O0">, Group<O_Group>, Flags<[CC1Option, HelpHidden]>;
 def O4 : Flag<["-"], "O4">, Group<O_Group>, Flags<[CC1Option, HelpHidden]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to