Skip to content

Commit e6de86c

Browse files
aeubankstru
aeubanks
authored and
tru
committed
Revert "Reland [clang] Canonicalize system headers in dependency file when -canonical-prefixes" (#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 #70011.
1 parent bb66d8f commit e6de86c

File tree

8 files changed

+12
-70
lines changed

8 files changed

+12
-70
lines changed

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6079,9 +6079,6 @@ let Flags = [CC1Option, NoDriverOption] in {
60796079
def sys_header_deps : Flag<["-"], "sys-header-deps">,
60806080
HelpText<"Include system headers in dependency output">,
60816081
MarshallingInfoFlag<DependencyOutputOpts<"IncludeSystemHeaders">>;
6082-
def canonical_system_headers : Flag<["-"], "canonical-system-headers">,
6083-
HelpText<"Canonicalize system headers in dependency output">,
6084-
MarshallingInfoFlag<DependencyOutputOpts<"CanonicalSystemHeaders">>;
60856082
def module_file_deps : Flag<["-"], "module-file-deps">,
60866083
HelpText<"Include module files in dependency output">,
60876084
MarshallingInfoFlag<DependencyOutputOpts<"IncludeModuleFiles">>;

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ enum ExtraDepKind {
3434
class DependencyOutputOptions {
3535
public:
3636
unsigned IncludeSystemHeaders : 1; ///< Include system header dependencies.
37-
unsigned
38-
CanonicalSystemHeaders : 1; ///< canonicalize system header dependencies.
3937
unsigned ShowHeaderIncludes : 1; ///< Show header inclusions (-H).
4038
unsigned UsePhonyTargets : 1; ///< Include phony targets for each
4139
/// dependency, which can avoid some 'make'
@@ -84,11 +82,10 @@ class DependencyOutputOptions {
8482

8583
public:
8684
DependencyOutputOptions()
87-
: IncludeSystemHeaders(0), CanonicalSystemHeaders(0),
88-
ShowHeaderIncludes(0), UsePhonyTargets(0), AddMissingHeaderDeps(0),
89-
IncludeModuleFiles(0), ShowSkippedHeaderIncludes(0),
90-
HeaderIncludeFormat(HIFMT_Textual), HeaderIncludeFiltering(HIFIL_None) {
91-
}
85+
: IncludeSystemHeaders(0), ShowHeaderIncludes(0), UsePhonyTargets(0),
86+
AddMissingHeaderDeps(0), IncludeModuleFiles(0),
87+
ShowSkippedHeaderIncludes(0), HeaderIncludeFormat(HIFMT_Textual),
88+
HeaderIncludeFiltering(HIFIL_None) {}
9289
};
9390

9491
} // 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
@@ -1152,9 +1152,6 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
11521152
if (ArgM->getOption().matches(options::OPT_M) ||
11531153
ArgM->getOption().matches(options::OPT_MD))
11541154
CmdArgs.push_back("-sys-header-deps");
1155-
if (Args.hasFlag(options::OPT_canonical_prefixes,
1156-
options::OPT_no_canonical_prefixes, true))
1157-
CmdArgs.push_back("-canonical-system-headers");
11581155
if ((isa<PrecompileJobAction>(JA) &&
11591156
!Args.hasArg(options::OPT_fno_module_file_deps)) ||
11601157
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, const FileEntry &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->getFile(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)