Skip to content

Commit

Permalink
Fix emission of phony dependency targets when adding extra deps
Browse files Browse the repository at this point in the history
Summary:
This commit fixes a bug where passing extra dependency entries
(using -fdepfile-entry) would result in -MP incorrectly emitting
a phony target for the input file, and no phony target for the
first extra dependency.

The extra dependencies are added first to the filename vector in
DFGImpl. That clashed with the emission of the phony targets, as
the code assumed that the first index always correspond to the
input file.

Reviewers: rsmith, pcc, krasin, bruno, vsapsai

Reviewed By: vsapsai

Subscribers: vsapsai, bruno, cfe-commits

Differential Revision: https://reviews.llvm.org/D44568

llvm-svn: 333413
  • Loading branch information
dstenb committed May 29, 2018
1 parent f6c870a commit d45d7ea
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 7 deletions.
22 changes: 15 additions & 7 deletions clang/lib/Frontend/DependencyFile.cpp
Expand Up @@ -163,6 +163,7 @@ class DFGImpl : public PPCallbacks {
bool SeenMissingHeader;
bool IncludeModuleFiles;
DependencyOutputFormat OutputFormat;
unsigned InputFileIndex;

private:
bool FileMatchesDepCriteria(const char *Filename,
Expand All @@ -177,9 +178,11 @@ class DFGImpl : public PPCallbacks {
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
OutputFormat(Opts.OutputFormat) {
OutputFormat(Opts.OutputFormat),
InputFileIndex(0) {
for (const auto &ExtraDep : Opts.ExtraDeps) {
AddFilename(ExtraDep);
if (AddFilename(ExtraDep))
++InputFileIndex;
}
}

Expand All @@ -201,7 +204,7 @@ class DFGImpl : public PPCallbacks {
OutputDependencyFile();
}

void AddFilename(StringRef Filename);
bool AddFilename(StringRef Filename);
bool includeSystemHeaders() const { return IncludeSystemHeaders; }
bool includeModuleFiles() const { return IncludeModuleFiles; }
};
Expand Down Expand Up @@ -325,9 +328,12 @@ void DFGImpl::InclusionDirective(SourceLocation HashLoc,
}
}

void DFGImpl::AddFilename(StringRef Filename) {
if (FilesSet.insert(Filename).second)
bool DFGImpl::AddFilename(StringRef Filename) {
if (FilesSet.insert(Filename).second) {
Files.push_back(Filename);
return true;
}
return false;
}

/// Print the filename, with escaping or quoting that accommodates the three
Expand Down Expand Up @@ -463,8 +469,10 @@ void DFGImpl::OutputDependencyFile() {

// Create phony targets if requested.
if (PhonyTarget && !Files.empty()) {
// Skip the first entry, this is always the input file itself.
for (auto I = Files.begin() + 1, E = Files.end(); I != E; ++I) {
unsigned Index = 0;
for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
if (Index++ == InputFileIndex)
continue;
OS << '\n';
PrintFilename(OS, *I, OutputFormat);
OS << ":\n";
Expand Down
10 changes: 10 additions & 0 deletions clang/test/Frontend/dependency-gen-extradeps-phony.c
@@ -0,0 +1,10 @@
// RUN: %clang -MM -MP -Xclang -fdepfile-entry=1.extra -Xclang -fdepfile-entry=2.extra -Xclang -fdepfile-entry=2.extra %s | \
// RUN: FileCheck %s --implicit-check-not=.c:
//
// Verify that phony targets are only created for the extra dependency files,
// and not the input file.

// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra \
// CHECK-NEXT: dependency-gen-extradeps-phony.c
// CHECK: 1.extra:
// CHECK: 2.extra:

0 comments on commit d45d7ea

Please sign in to comment.