Skip to content

Commit 7acad9a

Browse files
aeubanksbenlangmuir
aeubanks
authored and
benlangmuir
committed
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. (cherry picked from commit 955dd88) rdar://118178759
1 parent 5462890 commit 7acad9a

File tree

9 files changed

+13
-74
lines changed

9 files changed

+13
-74
lines changed

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6183,9 +6183,6 @@ let Flags = [CC1Option, NoDriverOption] in {
61836183
def sys_header_deps : Flag<["-"], "sys-header-deps">,
61846184
HelpText<"Include system headers in dependency output">,
61856185
MarshallingInfoFlag<DependencyOutputOpts<"IncludeSystemHeaders">>;
6186-
def canonical_system_headers : Flag<["-"], "canonical-system-headers">,
6187-
HelpText<"Canonicalize system headers in dependency output">,
6188-
MarshallingInfoFlag<DependencyOutputOpts<"CanonicalSystemHeaders">>;
61896186
def skip_unused_modulemap_file_deps : Flag<["-"], "skip-unused-modulemap-deps">,
61906187
HelpText<"Include module map files only for imported modules in dependency output">,
61916188
MarshallingInfoFlag<DependencyOutputOpts<"SkipUnusedModuleMaps">>;

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

Lines changed: 2 additions & 5 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'
@@ -85,9 +83,8 @@ class DependencyOutputOptions {
8583

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

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ class ExternalSemaSource;
4343
class FrontendOptions;
4444
class PCHContainerReader;
4545
class Preprocessor;
46-
class FileManager;
4746
class PreprocessorOptions;
4847
class PreprocessorOutputOptions;
4948

@@ -82,14 +81,11 @@ class DependencyCollector {
8281
/// Return true if system files should be passed to sawDependency().
8382
virtual bool needSystemDependencies() { return false; }
8483

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

9490
protected:
9591
/// Return true if the filename was added to the list of dependencies, false
@@ -123,10 +119,6 @@ class DependencyFileGenerator : public DependencyCollector {
123119
bool sawDependency(StringRef Filename, bool FromModule, bool IsSystem,
124120
bool IsModuleFile, bool IsMissing) final;
125121

126-
bool shouldCanonicalizeSystemDependencies() override {
127-
return CanonicalSystemHeaders;
128-
}
129-
130122
protected:
131123
void outputDependencyFile(llvm::raw_ostream &OS);
132124

@@ -137,7 +129,6 @@ class DependencyFileGenerator : public DependencyCollector {
137129
std::string OutputFile;
138130
std::vector<std::string> Targets;
139131
bool IncludeSystemHeaders;
140-
bool CanonicalSystemHeaders;
141132
bool PhonyTarget;
142133
bool AddMissingHeaderDeps;
143134
bool SeenMissingHeader;

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,9 +1154,6 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
11541154
if (ArgM->getOption().matches(options::OPT_M) ||
11551155
ArgM->getOption().matches(options::OPT_MD))
11561156
CmdArgs.push_back("-sys-header-deps");
1157-
if (Args.hasFlag(options::OPT_canonical_prefixes,
1158-
options::OPT_no_canonical_prefixes, true))
1159-
CmdArgs.push_back("-canonical-system-headers");
11601157
if ((isa<PrecompileJobAction>(JA) &&
11611158
!Args.hasArg(options::OPT_fno_module_file_deps)) ||
11621159
Args.hasArg(options::OPT_fmodule_file_deps))

‎clang/lib/Frontend/DependencyFile.cpp

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

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

@@ -73,11 +70,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
7370
StringRef RelativePath, const Module *Imported,
7471
SrcMgr::CharacteristicKind FileType) override {
7572
if (!File)
76-
DepCollector.maybeAddDependency(FileName,
77-
/*FromModule*/ false,
73+
DepCollector.maybeAddDependency(FileName, /*FromModule*/ false,
7874
/*IsSystem*/ false,
7975
/*IsModuleFile*/ false,
80-
&PP.getFileManager(),
8176
/*IsMissing*/ true);
8277
// Files that actually exist are handled by FileChanged.
8378
}
@@ -89,11 +84,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
8984
return;
9085
StringRef Filename =
9186
llvm::sys::path::remove_leading_dotslash(File->getName());
92-
DepCollector.maybeAddDependency(Filename,
93-
/*FromModule=*/false,
87+
DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
9488
/*IsSystem=*/isSystem(FileType),
9589
/*IsModuleFile=*/false,
96-
&PP.getFileManager(),
9790
/*IsMissing=*/false);
9891
}
9992

@@ -109,11 +102,9 @@ struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
109102
void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
110103
bool IsSystem) override {
111104
StringRef Filename = Entry.getName();
112-
DepCollector.maybeAddDependency(Filename,
113-
/*FromModule*/ false,
105+
DepCollector.maybeAddDependency(Filename, /*FromModule*/ false,
114106
/*IsSystem*/ IsSystem,
115107
/*IsModuleFile*/ false,
116-
/*FileMgr*/ nullptr,
117108
/*IsMissing*/ false);
118109
}
119110
};
@@ -134,7 +125,6 @@ struct DFGMMCallback : public ModuleMapCallbacks {
134125
DepCollector.maybeAddDependency(Filename, /*FromModule*/ false,
135126
/*IsSystem*/ IsSystem,
136127
/*IsModuleFile*/ false,
137-
/*FileMgr*/nullptr,
138128
/*IsMissing*/ false);
139129
}
140130

@@ -145,7 +135,6 @@ struct DFGMMCallback : public ModuleMapCallbacks {
145135
DepCollector.maybeAddDependency(Entry.getName(), /*FromModule*/ false,
146136
/*IsSystem*/ IsSystem,
147137
/*IsModuleFile*/ false,
148-
/*FileMgr*/nullptr,
149138
/*IsMissing*/ false);
150139
}
151140
};
@@ -161,10 +150,8 @@ struct DepCollectorASTListener : public ASTReaderListener {
161150
}
162151
void visitModuleFile(StringRef Filename,
163152
serialization::ModuleKind Kind) override {
164-
DepCollector.maybeAddDependency(Filename,
165-
/*FromModule*/ true,
153+
DepCollector.maybeAddDependency(Filename, /*FromModule*/ true,
166154
/*IsSystem*/ false, /*IsModuleFile*/ true,
167-
/*FileMgr*/ nullptr,
168155
/*IsMissing*/ false);
169156
}
170157
bool visitInputFile(StringRef Filename, bool IsSystem,
@@ -178,7 +165,7 @@ struct DepCollectorASTListener : public ASTReaderListener {
178165
Filename = FE->getName();
179166

180167
DepCollector.maybeAddDependency(Filename, /*FromModule*/ true, IsSystem,
181-
/*IsModuleFile*/ false, /*FileMgr*/ nullptr,
168+
/*IsModuleFile*/ false,
182169
/*IsMissing*/ false);
183170
return true;
184171
}
@@ -188,15 +175,9 @@ struct DepCollectorASTListener : public ASTReaderListener {
188175
void DependencyCollector::maybeAddDependency(StringRef Filename,
189176
bool FromModule, bool IsSystem,
190177
bool IsModuleFile,
191-
FileManager *FileMgr,
192178
bool IsMissing) {
193-
if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing)) {
194-
if (IsSystem && FileMgr && shouldCanonicalizeSystemDependencies()) {
195-
if (auto F = FileMgr->getFile(Filename))
196-
Filename = FileMgr->getCanonicalName(*F);
197-
}
179+
if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
198180
addDependency(Filename);
199-
}
200181
}
201182

202183
bool DependencyCollector::addDependency(StringRef Filename) {
@@ -245,7 +226,6 @@ DependencyFileGenerator::DependencyFileGenerator(
245226
IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OB)
246227
: OutputBackend(std::move(OB)), OutputFile(Opts.OutputFile),
247228
Targets(Opts.Targets), IncludeSystemHeaders(Opts.IncludeSystemHeaders),
248-
CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
249229
PhonyTarget(Opts.UsePhonyTargets),
250230
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), SeenMissingHeader(false),
251231
IncludeModuleFiles(Opts.IncludeModuleFiles),

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,18 +176,17 @@ class ReversePrefixMappingDependencyFileGenerator
176176
}
177177

178178
void maybeAddDependency(StringRef Filename, bool FromModule, bool IsSystem,
179-
bool IsModuleFile, FileManager *FileMgr,
180-
bool IsMissing) override {
179+
bool IsModuleFile, bool IsMissing) override {
181180
if (ReverseMapper.empty())
182181
return DependencyFileGenerator::maybeAddDependency(
183-
Filename, FromModule, IsSystem, IsModuleFile, FileMgr, IsMissing);
182+
Filename, FromModule, IsSystem, IsModuleFile, IsMissing);
184183

185184
// We may get canonicalized paths if prefix headers/PCH are used, so make
186185
// sure to remap them back to original source paths.
187186
SmallString<256> New{Filename};
188187
ReverseMapper.mapInPlace(New);
189188
return DependencyFileGenerator::maybeAddDependency(
190-
New, FromModule, IsSystem, IsModuleFile, FileMgr, IsMissing);
189+
New, FromModule, IsSystem, IsModuleFile, IsMissing);
191190
}
192191
};
193192

‎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)