Skip to content

Commit 955dd88

Browse files
aeubanksaeubanks
authored
Revert "Reland [clang] Canonicalize system headers in dependency file when -canonical-prefixes" (llvm#71697)
This reverts commit 578a471. This causes multiple issues. Compile time slowdown due to more path canonicalization, and weird behavior on Windows. Will reland under a separate flag `-f[no-]canonical-system-headers` to match gcc in the future and further limit when it's passed by default. Fixes llvm#70011.
1 parent 5447017 commit 955dd88

File tree

8 files changed

+12
-71
lines changed

8 files changed

+12
-71
lines changed

‎clang/include/clang/Driver/Options.td

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6954,9 +6954,6 @@ let Visibility = [CC1Option] in {
69546954
def sys_header_deps : Flag<["-"], "sys-header-deps">,
69556955
HelpText<"Include system headers in dependency output">,
69566956
MarshallingInfoFlag<DependencyOutputOpts<"IncludeSystemHeaders">>;
6957-
def canonical_system_headers : Flag<["-"], "canonical-system-headers">,
6958-
HelpText<"Canonicalize system headers in dependency output">,
6959-
MarshallingInfoFlag<DependencyOutputOpts<"CanonicalSystemHeaders">>;
69606957
def module_file_deps : Flag<["-"], "module-file-deps">,
69616958
HelpText<"Include module files in dependency output">,
69626959
MarshallingInfoFlag<DependencyOutputOpts<"IncludeModuleFiles">>;

‎clang/include/clang/Frontend/DependencyOutputOptions.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ class DependencyOutputOptions {
3636
LLVM_PREFERRED_TYPE(bool)
3737
unsigned IncludeSystemHeaders : 1; ///< Include system header dependencies.
3838
LLVM_PREFERRED_TYPE(bool)
39-
unsigned
40-
CanonicalSystemHeaders : 1; ///< canonicalize system header dependencies.
41-
LLVM_PREFERRED_TYPE(bool)
4239
unsigned ShowHeaderIncludes : 1; ///< Show header inclusions (-H).
4340
LLVM_PREFERRED_TYPE(bool)
4441
unsigned UsePhonyTargets : 1; ///< Include phony targets for each
@@ -91,11 +88,10 @@ class DependencyOutputOptions {
9188

9289
public:
9390
DependencyOutputOptions()
94-
: IncludeSystemHeaders(0), CanonicalSystemHeaders(0),
95-
ShowHeaderIncludes(0), UsePhonyTargets(0), AddMissingHeaderDeps(0),
96-
IncludeModuleFiles(0), ShowSkippedHeaderIncludes(0),
97-
HeaderIncludeFormat(HIFMT_Textual), HeaderIncludeFiltering(HIFIL_None) {
98-
}
91+
: IncludeSystemHeaders(0), ShowHeaderIncludes(0), UsePhonyTargets(0),
92+
AddMissingHeaderDeps(0), IncludeModuleFiles(0),
93+
ShowSkippedHeaderIncludes(0), HeaderIncludeFormat(HIFMT_Textual),
94+
HeaderIncludeFiltering(HIFIL_None) {}
9995
};
10096

10197
} // end namespace clang

‎clang/include/clang/Frontend/Utils.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ class ExternalSemaSource;
4141
class FrontendOptions;
4242
class PCHContainerReader;
4343
class Preprocessor;
44-
class FileManager;
4544
class PreprocessorOptions;
4645
class PreprocessorOutputOptions;
4746

@@ -80,14 +79,11 @@ class DependencyCollector {
8079
/// Return true if system files should be passed to sawDependency().
8180
virtual bool needSystemDependencies() { return false; }
8281

83-
/// Return true if system files should be canonicalized.
84-
virtual bool shouldCanonicalizeSystemDependencies() { return false; }
85-
8682
/// Add a dependency \p Filename if it has not been seen before and
8783
/// sawDependency() returns true.
8884
virtual void maybeAddDependency(StringRef Filename, bool FromModule,
8985
bool IsSystem, bool IsModuleFile,
90-
FileManager *FileMgr, bool IsMissing);
86+
bool IsMissing);
9187

9288
protected:
9389
/// Return true if the filename was added to the list of dependencies, false
@@ -116,10 +112,6 @@ class DependencyFileGenerator : public DependencyCollector {
116112
bool sawDependency(StringRef Filename, bool FromModule, bool IsSystem,
117113
bool IsModuleFile, bool IsMissing) final;
118114

119-
bool shouldCanonicalizeSystemDependencies() override {
120-
return CanonicalSystemHeaders;
121-
}
122-
123115
protected:
124116
void outputDependencyFile(llvm::raw_ostream &OS);
125117

@@ -129,7 +121,6 @@ class DependencyFileGenerator : public DependencyCollector {
129121
std::string OutputFile;
130122
std::vector<std::string> Targets;
131123
bool IncludeSystemHeaders;
132-
bool CanonicalSystemHeaders;
133124
bool PhonyTarget;
134125
bool AddMissingHeaderDeps;
135126
bool SeenMissingHeader;

‎clang/lib/Driver/ToolChains/Clang.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,9 +1183,6 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
11831183
if (ArgM->getOption().matches(options::OPT_M) ||
11841184
ArgM->getOption().matches(options::OPT_MD))
11851185
CmdArgs.push_back("-sys-header-deps");
1186-
if (Args.hasFlag(options::OPT_canonical_prefixes,
1187-
options::OPT_no_canonical_prefixes, true))
1188-
CmdArgs.push_back("-canonical-system-headers");
11891186
if ((isa<PrecompileJobAction>(JA) &&
11901187
!Args.hasArg(options::OPT_fno_module_file_deps)) ||
11911188
Args.hasArg(options::OPT_fmodule_file_deps))

‎clang/lib/Frontend/DependencyFile.cpp

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,16 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
4949
DepCollector.maybeAddDependency(
5050
llvm::sys::path::remove_leading_dotslash(*Filename),
5151
/*FromModule*/ false, isSystem(FileType), /*IsModuleFile*/ false,
52-
&PP.getFileManager(),
5352
/*IsMissing*/ false);
5453
}
5554

5655
void FileSkipped(const FileEntryRef &SkippedFile, const Token &FilenameTok,
5756
SrcMgr::CharacteristicKind FileType) override {
5857
StringRef Filename =
5958
llvm::sys::path::remove_leading_dotslash(SkippedFile.getName());
60-
DepCollector.maybeAddDependency(Filename,
61-
/*FromModule=*/false,
59+
DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
6260
/*IsSystem=*/isSystem(FileType),
6361
/*IsModuleFile=*/false,
64-
&PP.getFileManager(),
6562
/*IsMissing=*/false);
6663
}
6764

@@ -72,11 +69,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
7269
StringRef RelativePath, const Module *Imported,
7370
SrcMgr::CharacteristicKind FileType) override {
7471
if (!File)
75-
DepCollector.maybeAddDependency(FileName,
76-
/*FromModule*/ false,
72+
DepCollector.maybeAddDependency(FileName, /*FromModule*/ false,
7773
/*IsSystem*/ false,
7874
/*IsModuleFile*/ false,
79-
&PP.getFileManager(),
8075
/*IsMissing*/ true);
8176
// Files that actually exist are handled by FileChanged.
8277
}
@@ -88,11 +83,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
8883
return;
8984
StringRef Filename =
9085
llvm::sys::path::remove_leading_dotslash(File->getName());
91-
DepCollector.maybeAddDependency(Filename,
92-
/*FromModule=*/false,
86+
DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
9387
/*IsSystem=*/isSystem(FileType),
9488
/*IsModuleFile=*/false,
95-
&PP.getFileManager(),
9689
/*IsMissing=*/false);
9790
}
9891

@@ -108,11 +101,9 @@ struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
108101
void moduleMapFileRead(SourceLocation Loc, FileEntryRef Entry,
109102
bool IsSystem) override {
110103
StringRef Filename = Entry.getName();
111-
DepCollector.maybeAddDependency(Filename,
112-
/*FromModule*/ false,
104+
DepCollector.maybeAddDependency(Filename, /*FromModule*/ false,
113105
/*IsSystem*/ IsSystem,
114106
/*IsModuleFile*/ false,
115-
/*FileMgr*/ nullptr,
116107
/*IsMissing*/ false);
117108
}
118109
};
@@ -128,10 +119,8 @@ struct DepCollectorASTListener : public ASTReaderListener {
128119
}
129120
void visitModuleFile(StringRef Filename,
130121
serialization::ModuleKind Kind) override {
131-
DepCollector.maybeAddDependency(Filename,
132-
/*FromModule*/ true,
122+
DepCollector.maybeAddDependency(Filename, /*FromModule*/ true,
133123
/*IsSystem*/ false, /*IsModuleFile*/ true,
134-
/*FileMgr*/ nullptr,
135124
/*IsMissing*/ false);
136125
}
137126
bool visitInputFile(StringRef Filename, bool IsSystem,
@@ -145,7 +134,7 @@ struct DepCollectorASTListener : public ASTReaderListener {
145134
Filename = FE->getName();
146135

147136
DepCollector.maybeAddDependency(Filename, /*FromModule*/ true, IsSystem,
148-
/*IsModuleFile*/ false, /*FileMgr*/ nullptr,
137+
/*IsModuleFile*/ false,
149138
/*IsMissing*/ false);
150139
return true;
151140
}
@@ -155,15 +144,9 @@ struct DepCollectorASTListener : public ASTReaderListener {
155144
void DependencyCollector::maybeAddDependency(StringRef Filename,
156145
bool FromModule, bool IsSystem,
157146
bool IsModuleFile,
158-
FileManager *FileMgr,
159147
bool IsMissing) {
160-
if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing)) {
161-
if (IsSystem && FileMgr && shouldCanonicalizeSystemDependencies()) {
162-
if (auto F = FileMgr->getOptionalFileRef(Filename))
163-
Filename = FileMgr->getCanonicalName(*F);
164-
}
148+
if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
165149
addDependency(Filename);
166-
}
167150
}
168151

169152
bool DependencyCollector::addDependency(StringRef Filename) {
@@ -211,7 +194,6 @@ DependencyFileGenerator::DependencyFileGenerator(
211194
const DependencyOutputOptions &Opts)
212195
: OutputFile(Opts.OutputFile), Targets(Opts.Targets),
213196
IncludeSystemHeaders(Opts.IncludeSystemHeaders),
214-
CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
215197
PhonyTarget(Opts.UsePhonyTargets),
216198
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), SeenMissingHeader(false),
217199
IncludeModuleFiles(Opts.IncludeModuleFiles),

‎clang/test/Driver/canonical-system-headers.c

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎clang/test/Preprocessor/Inputs/canonical-system-headers/a.h

Whitespace-only changes.

‎clang/test/Preprocessor/canonical-system-headers.c

Lines changed: 0 additions & 16 deletions
This file was deleted.

0 commit comments

Comments
 (0)