Lekensteyn updated this revision to Diff 115946.
Lekensteyn retitled this revision from "Expand absolute system header paths
when using -MD depfiles" to "Try to shorten system header paths when using -MD
depfiles".
Lekensteyn edited the summary of this revision.
Lekensteyn added a comment.
Thanks for the feedback, I have the patch accordingly. Changes in v2:
- Add (no-op) -fcanonical-system-headers option as suggested by @thakis (yes,
GCC supports this option, but GCC also has a configure option that can change
the default which Clang does not have)
- Check whether path is shorter instead of checking for absolute path (matches
the original GCC bug, should address @joerg's concern)
- Update tests to use "%t" instead of "%T" (noticed
https://reviews.llvm.org/D35396 after looking for whether "mkdir -p" is
allowed) + other minor text/redundancy tweaks
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,33 @@
+// RUN: mkdir -p %t/sub/dir
+// RUN: echo > %t/sub/empty.h
+
+// Test that system header paths are expanded
+//
+// 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 system header paths are not expanded to a longer form
+//
+// 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 user header paths are not expanded
+//
+// RUN: %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 system header paths are not expanded with -fno-canonical-system-headers
+// (and also that the -fsystem-system-headers option is accepted)
+//
+// RUN: %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %t/sub/dir/.. -fcanonical-system-headers -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,9 @@
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 -f[no-]canonical-system-headers.
+ if (!Arg.startswith("-M") && Arg != "-fno-canonical-system-headers" &&
+ Arg != "-fcanonical-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,15 @@
if (!FileMatchesDepCriteria(Filename.data(), FileType))
return;
+ // Try to shorten system header paths like GCC does (unless
+ // -fno-canonical-system-headers is given).
+ if (CanonicalSystemHeaders && isSystem(FileType)) {
+ StringRef RealPath = FE->tryGetRealPathName();
+ if (!RealPath.empty() && RealPath.size() < Filename.size()) {
+ 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,13 @@
Args.AddLastArg(CmdArgs, options::OPT_C);
Args.AddLastArg(CmdArgs, options::OPT_CC);
+ if (Arg *A = Args.getLastArg(options::OPT_fno_canonical_system_headers,
+ options::OPT_fcanonical_system_headers)) {
+ if (A->getOption().matches(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
@@ -73,8 +73,8 @@
// 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("-M", "-MM", "-MG", "-MP", "-MD", "-MMD", true)
+ .Cases("-fno-canonical-system-headers", "-fcanonical-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; ///< Try to output a shorter 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,11 @@
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 fcanonical_system_headers : Flag<["-"], "fcanonical-system-headers">,
+ Group<M_Group>;
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
