[clang] Revert "Reland [clang] Canonicalize system headers in dependency file when -canonical-prefixes" (PR #71697)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 8 08:15:23 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang
Author: Arthur Eubanks (aeubanks)
<details>
<summary>Changes</summary>
This reverts commit 578a4716f549167165a2ec3bac89c86706136d4e.
This causes multiple issues. Compile time slowdown due to more path canonicalization, and weird behavior on Windows.
Fixes #<!-- -->70011.
---
Full diff: https://github.com/llvm/llvm-project/pull/71697.diff
8 Files Affected:
- (modified) clang/include/clang/Driver/Options.td (-3)
- (modified) clang/include/clang/Frontend/DependencyOutputOptions.h (+4-8)
- (modified) clang/include/clang/Frontend/Utils.h (+1-10)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (-3)
- (modified) clang/lib/Frontend/DependencyFile.cpp (+7-25)
- (removed) clang/test/Driver/canonical-system-headers.c (-6)
- (removed) clang/test/Preprocessor/Inputs/canonical-system-headers/a.h ()
- (removed) clang/test/Preprocessor/canonical-system-headers.c (-16)
``````````diff
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 36052511203f65c..7d933aabd16d7fa 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6954,9 +6954,6 @@ let Visibility = [CC1Option] in {
def sys_header_deps : Flag<["-"], "sys-header-deps">,
HelpText<"Include system headers in dependency output">,
MarshallingInfoFlag<DependencyOutputOpts<"IncludeSystemHeaders">>;
-def canonical_system_headers : Flag<["-"], "canonical-system-headers">,
- HelpText<"Canonicalize system headers in dependency output">,
- MarshallingInfoFlag<DependencyOutputOpts<"CanonicalSystemHeaders">>;
def module_file_deps : Flag<["-"], "module-file-deps">,
HelpText<"Include module files in dependency output">,
MarshallingInfoFlag<DependencyOutputOpts<"IncludeModuleFiles">>;
diff --git a/clang/include/clang/Frontend/DependencyOutputOptions.h b/clang/include/clang/Frontend/DependencyOutputOptions.h
index 558d8a0a0ab693b..d92a87d78d7c5a0 100644
--- a/clang/include/clang/Frontend/DependencyOutputOptions.h
+++ b/clang/include/clang/Frontend/DependencyOutputOptions.h
@@ -36,9 +36,6 @@ class DependencyOutputOptions {
LLVM_PREFERRED_TYPE(bool)
unsigned IncludeSystemHeaders : 1; ///< Include system header dependencies.
LLVM_PREFERRED_TYPE(bool)
- unsigned
- CanonicalSystemHeaders : 1; ///< canonicalize system header dependencies.
- LLVM_PREFERRED_TYPE(bool)
unsigned ShowHeaderIncludes : 1; ///< Show header inclusions (-H).
LLVM_PREFERRED_TYPE(bool)
unsigned UsePhonyTargets : 1; ///< Include phony targets for each
@@ -91,11 +88,10 @@ class DependencyOutputOptions {
public:
DependencyOutputOptions()
- : IncludeSystemHeaders(0), CanonicalSystemHeaders(0),
- ShowHeaderIncludes(0), UsePhonyTargets(0), AddMissingHeaderDeps(0),
- IncludeModuleFiles(0), ShowSkippedHeaderIncludes(0),
- HeaderIncludeFormat(HIFMT_Textual), HeaderIncludeFiltering(HIFIL_None) {
- }
+ : IncludeSystemHeaders(0), ShowHeaderIncludes(0), UsePhonyTargets(0),
+ AddMissingHeaderDeps(0), IncludeModuleFiles(0),
+ ShowSkippedHeaderIncludes(0), HeaderIncludeFormat(HIFMT_Textual),
+ HeaderIncludeFiltering(HIFIL_None) {}
};
} // end namespace clang
diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h
index 8300e45d15fe5f6..143cf4359f00b50 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -41,7 +41,6 @@ class ExternalSemaSource;
class FrontendOptions;
class PCHContainerReader;
class Preprocessor;
-class FileManager;
class PreprocessorOptions;
class PreprocessorOutputOptions;
@@ -80,14 +79,11 @@ class DependencyCollector {
/// Return true if system files should be passed to sawDependency().
virtual bool needSystemDependencies() { return false; }
- /// Return true if system files should be canonicalized.
- virtual bool shouldCanonicalizeSystemDependencies() { return false; }
-
/// Add a dependency \p Filename if it has not been seen before and
/// sawDependency() returns true.
virtual void maybeAddDependency(StringRef Filename, bool FromModule,
bool IsSystem, bool IsModuleFile,
- FileManager *FileMgr, bool IsMissing);
+ bool IsMissing);
protected:
/// Return true if the filename was added to the list of dependencies, false
@@ -116,10 +112,6 @@ class DependencyFileGenerator : public DependencyCollector {
bool sawDependency(StringRef Filename, bool FromModule, bool IsSystem,
bool IsModuleFile, bool IsMissing) final;
- bool shouldCanonicalizeSystemDependencies() override {
- return CanonicalSystemHeaders;
- }
-
protected:
void outputDependencyFile(llvm::raw_ostream &OS);
@@ -129,7 +121,6 @@ class DependencyFileGenerator : public DependencyCollector {
std::string OutputFile;
std::vector<std::string> Targets;
bool IncludeSystemHeaders;
- bool CanonicalSystemHeaders;
bool PhonyTarget;
bool AddMissingHeaderDeps;
bool SeenMissingHeader;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 22f992166ded6c0..4f03131c535c9b6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1183,9 +1183,6 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
if (ArgM->getOption().matches(options::OPT_M) ||
ArgM->getOption().matches(options::OPT_MD))
CmdArgs.push_back("-sys-header-deps");
- if (Args.hasFlag(options::OPT_canonical_prefixes,
- options::OPT_no_canonical_prefixes, true))
- CmdArgs.push_back("-canonical-system-headers");
if ((isa<PrecompileJobAction>(JA) &&
!Args.hasArg(options::OPT_fno_module_file_deps)) ||
Args.hasArg(options::OPT_fmodule_file_deps))
diff --git a/clang/lib/Frontend/DependencyFile.cpp b/clang/lib/Frontend/DependencyFile.cpp
index c2f6f41ae291efb..19abcac2befbdd1 100644
--- a/clang/lib/Frontend/DependencyFile.cpp
+++ b/clang/lib/Frontend/DependencyFile.cpp
@@ -49,7 +49,6 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
DepCollector.maybeAddDependency(
llvm::sys::path::remove_leading_dotslash(*Filename),
/*FromModule*/ false, isSystem(FileType), /*IsModuleFile*/ false,
- &PP.getFileManager(),
/*IsMissing*/ false);
}
@@ -57,11 +56,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
SrcMgr::CharacteristicKind FileType) override {
StringRef Filename =
llvm::sys::path::remove_leading_dotslash(SkippedFile.getName());
- DepCollector.maybeAddDependency(Filename,
- /*FromModule=*/false,
+ DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
/*IsSystem=*/isSystem(FileType),
/*IsModuleFile=*/false,
- &PP.getFileManager(),
/*IsMissing=*/false);
}
@@ -72,11 +69,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
StringRef RelativePath, const Module *Imported,
SrcMgr::CharacteristicKind FileType) override {
if (!File)
- DepCollector.maybeAddDependency(FileName,
- /*FromModule*/ false,
+ DepCollector.maybeAddDependency(FileName, /*FromModule*/ false,
/*IsSystem*/ false,
/*IsModuleFile*/ false,
- &PP.getFileManager(),
/*IsMissing*/ true);
// Files that actually exist are handled by FileChanged.
}
@@ -88,11 +83,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
return;
StringRef Filename =
llvm::sys::path::remove_leading_dotslash(File->getName());
- DepCollector.maybeAddDependency(Filename,
- /*FromModule=*/false,
+ DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
/*IsSystem=*/isSystem(FileType),
/*IsModuleFile=*/false,
- &PP.getFileManager(),
/*IsMissing=*/false);
}
@@ -108,11 +101,9 @@ struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
void moduleMapFileRead(SourceLocation Loc, FileEntryRef Entry,
bool IsSystem) override {
StringRef Filename = Entry.getName();
- DepCollector.maybeAddDependency(Filename,
- /*FromModule*/ false,
+ DepCollector.maybeAddDependency(Filename, /*FromModule*/ false,
/*IsSystem*/ IsSystem,
/*IsModuleFile*/ false,
- /*FileMgr*/ nullptr,
/*IsMissing*/ false);
}
};
@@ -128,10 +119,8 @@ struct DepCollectorASTListener : public ASTReaderListener {
}
void visitModuleFile(StringRef Filename,
serialization::ModuleKind Kind) override {
- DepCollector.maybeAddDependency(Filename,
- /*FromModule*/ true,
+ DepCollector.maybeAddDependency(Filename, /*FromModule*/ true,
/*IsSystem*/ false, /*IsModuleFile*/ true,
- /*FileMgr*/ nullptr,
/*IsMissing*/ false);
}
bool visitInputFile(StringRef Filename, bool IsSystem,
@@ -145,7 +134,7 @@ struct DepCollectorASTListener : public ASTReaderListener {
Filename = FE->getName();
DepCollector.maybeAddDependency(Filename, /*FromModule*/ true, IsSystem,
- /*IsModuleFile*/ false, /*FileMgr*/ nullptr,
+ /*IsModuleFile*/ false,
/*IsMissing*/ false);
return true;
}
@@ -155,15 +144,9 @@ struct DepCollectorASTListener : public ASTReaderListener {
void DependencyCollector::maybeAddDependency(StringRef Filename,
bool FromModule, bool IsSystem,
bool IsModuleFile,
- FileManager *FileMgr,
bool IsMissing) {
- if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing)) {
- if (IsSystem && FileMgr && shouldCanonicalizeSystemDependencies()) {
- if (auto F = FileMgr->getOptionalFileRef(Filename))
- Filename = FileMgr->getCanonicalName(*F);
- }
+ if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
addDependency(Filename);
- }
}
bool DependencyCollector::addDependency(StringRef Filename) {
@@ -211,7 +194,6 @@ DependencyFileGenerator::DependencyFileGenerator(
const DependencyOutputOptions &Opts)
: OutputFile(Opts.OutputFile), Targets(Opts.Targets),
IncludeSystemHeaders(Opts.IncludeSystemHeaders),
- CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
PhonyTarget(Opts.UsePhonyTargets),
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
diff --git a/clang/test/Driver/canonical-system-headers.c b/clang/test/Driver/canonical-system-headers.c
deleted file mode 100644
index a7ab57521fc2249..000000000000000
--- a/clang/test/Driver/canonical-system-headers.c
+++ /dev/null
@@ -1,6 +0,0 @@
-// RUN: %clang -MD -no-canonical-prefixes -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO
-// RUN: %clang -MD -canonical-prefixes -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-YES
-// RUN: %clang -MD -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-YES
-
-// CHECK-YES: "-canonical-system-headers"
-// CHECK-NO-NOT: "-canonical-system-headers"
diff --git a/clang/test/Preprocessor/Inputs/canonical-system-headers/a.h b/clang/test/Preprocessor/Inputs/canonical-system-headers/a.h
deleted file mode 100644
index e69de29bb2d1d64..000000000000000
diff --git a/clang/test/Preprocessor/canonical-system-headers.c b/clang/test/Preprocessor/canonical-system-headers.c
deleted file mode 100644
index 0afa73c3e8225a9..000000000000000
--- a/clang/test/Preprocessor/canonical-system-headers.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// don't create symlinks on windows
-// UNSUPPORTED: system-windows
-// REQUIRES: shell
-
-// RUN: rm -rf %t
-// RUN: mkdir -p %t/foo/
-// RUN: ln -f -s %S/Inputs/canonical-system-headers %t/foo/include
-// RUN: %clang_cc1 -isystem %t/foo/include -sys-header-deps -MT foo.o -dependency-file %t2 %s -fsyntax-only
-// RUN: FileCheck %s --check-prefix=NOCANON --implicit-check-not=a.h < %t2
-// RUN: %clang_cc1 -isystem %t/foo/include -sys-header-deps -MT foo.o -dependency-file %t2 %s -fsyntax-only -canonical-system-headers
-// RUN: FileCheck %s --check-prefix=CANON --implicit-check-not=a.h < %t2
-
-// NOCANON: foo/include/a.h
-// CANON: Inputs/canonical-system-headers/a.h
-
-#include <a.h>
``````````
</details>
https://github.com/llvm/llvm-project/pull/71697
More information about the cfe-commits
mailing list