llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Ruoyu Zhong (ZhongRuoyu)
<details>
<summary>Changes</summary>
Currently, `llvm::sys::path::native` is called unconditionally when generating
dependency filenames. This is correct on Windows, where backslashes are valid
path separators, but can be incorrect on non-Windows platforms when the Clang
invocation is not MS-compatible (`-fno-ms-compatibility`), because in that case
backslashes in the filename are converted by `llvm::sys::path::native` to
forward slashes, while they should be treated as ordinary characters.
This fixes the following inconsistency on non-Windows platforms (notice how the
dependency output always converts the backslash to a forward slash, assuming
it's a path separator, while the actual include directive treats it as an
ordinary character when `-fno-ms-compatibility` is set):
```console
$ tree
.
└── foo
├── a
│ └── b.h
└── a\b.h
3 directories, 2 files
$ cat foo/a/b.h
#warning a/b.h
$ cat foo/a\\b.h
#warning a\\b.h
$ echo '#include "foo/a\\b.h"' | clang -c -xc - -fms-compatibility -o /dev/null
In file included from <stdin>:1:
./foo/a/b.h:1:2: warning: a/b.h [-W#warnings]
1 | #warning a/b.h
| ^
1 warning generated.
$ echo '#include "foo/a\\b.h"' | clang -xc - -fms-compatibility -M -MG
-.o: foo/a/b.h
$ echo '#include "foo/a\\b.h"' | clang -c -xc - -fno-ms-compatibility -o
/dev/null
In file included from <stdin>:1:
./foo/a\b.h:1:2: warning: a\\b.h [-W#warnings]
1 | #warning a\\b.h
| ^
1 warning generated.
$ echo '#include "foo/a\\b.h"' | clang -xc - -fno-ms-compatibility -M -MG
-.o: foo/a/b.h
```
See also: https://github.com/llvm/llvm-project/pull/160834.
---
Full diff: https://github.com/llvm/llvm-project/pull/160903.diff
3 Files Affected:
- (modified) clang/include/clang/Frontend/Utils.h (+4)
- (modified) clang/lib/Frontend/DependencyFile.cpp (+19-7)
- (added) clang/test/Frontend/dependency-gen-posix-ms-compatible.c (+17)
``````````diff
diff --git a/clang/include/clang/Frontend/Utils.h
b/clang/include/clang/Frontend/Utils.h
index f86c2f5074de0..6ffc200e61c23 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -118,6 +118,9 @@ class DependencyFileGenerator : public DependencyCollector {
void outputDependencyFile(llvm::raw_ostream &OS);
private:
+ void outputDependencyFilename(llvm::raw_ostream &OS,
+ StringRef Filename) const;
+
void outputDependencyFile(DiagnosticsEngine &Diags);
std::string OutputFile;
@@ -128,6 +131,7 @@ class DependencyFileGenerator : public DependencyCollector {
bool SeenMissingHeader;
bool IncludeModuleFiles;
DependencyOutputFormat OutputFormat;
+ bool MSCompatible;
unsigned InputFileIndex;
};
diff --git a/clang/lib/Frontend/DependencyFile.cpp
b/clang/lib/Frontend/DependencyFile.cpp
index 15fa7de35df97..90d2918000806 100644
--- a/clang/lib/Frontend/DependencyFile.cpp
+++ b/clang/lib/Frontend/DependencyFile.cpp
@@ -223,7 +223,7 @@ DependencyFileGenerator::DependencyFileGenerator(
PhonyTarget(Opts.UsePhonyTargets),
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
- OutputFormat(Opts.OutputFormat), InputFileIndex(0) {
+ OutputFormat(Opts.OutputFormat), MSCompatible(false), InputFileIndex(0) {
for (const auto &ExtraDep : Opts.ExtraDeps) {
if (addDependency(ExtraDep.first))
++InputFileIndex;
@@ -235,6 +235,8 @@ void
DependencyFileGenerator::attachToPreprocessor(Preprocessor &PP) {
if (AddMissingHeaderDeps)
PP.SetSuppressIncludeNotFoundError(true);
+ MSCompatible = PP.getLangOpts().MicrosoftExt;
+
DependencyCollector::attachToPreprocessor(PP);
}
@@ -312,11 +314,21 @@ void
DependencyFileGenerator::finishedMainFile(DiagnosticsEngine &Diags) {
/// https://msdn.microsoft.com/en-us/library/dd9y37ha.aspx for NMake info,
///
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
/// for Windows file-naming info.
-static void PrintFilename(raw_ostream &OS, StringRef Filename,
- DependencyOutputFormat OutputFormat) {
- // Convert filename to platform native path
+///
+/// Whether backslashes in the filename are treated as path separators or
+/// ordinary characters depends on whether the Clang invocation is
MS-compatible
+/// (-fms-compatibility). If it is, backslashes are treated as path separators,
+/// and are converted to the native path separator (backslash on Windows,
+/// forward slash on other platforms); if not, backslashes are treated as
+/// ordinary characters, and are not converted.
+void DependencyFileGenerator::outputDependencyFilename(
+ raw_ostream &OS, StringRef Filename) const {
llvm::SmallString<256> NativePath;
- llvm::sys::path::native(Filename.str(), NativePath);
+ if (MSCompatible) {
+ llvm::sys::path::native(Filename.str(), NativePath);
+ } else {
+ NativePath = Filename;
+ }
if (OutputFormat == DependencyOutputFormat::NMake) {
// Add quotes if needed. These are the characters listed as "special" to
@@ -400,7 +412,7 @@ void
DependencyFileGenerator::outputDependencyFile(llvm::raw_ostream &OS) {
Columns = 2;
}
OS << ' ';
- PrintFilename(OS, File, OutputFormat);
+ outputDependencyFilename(OS, File);
Columns += N + 1;
}
OS << '\n';
@@ -411,7 +423,7 @@ void
DependencyFileGenerator::outputDependencyFile(llvm::raw_ostream &OS) {
for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
if (Index++ == InputFileIndex)
continue;
- PrintFilename(OS, *I, OutputFormat);
+ outputDependencyFilename(OS, *I);
OS << ":\n";
}
}
diff --git a/clang/test/Frontend/dependency-gen-posix-ms-compatible.c
b/clang/test/Frontend/dependency-gen-posix-ms-compatible.c
new file mode 100644
index 0000000000000..208240b03a844
--- /dev/null
+++ b/clang/test/Frontend/dependency-gen-posix-ms-compatible.c
@@ -0,0 +1,17 @@
+// REQUIRES: !system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/include/foo
+// RUN: echo > %t.dir/include/foo/bar.h
+// RUN: echo > %t.dir/include/foo\\bar.h
+
+// RUN: %clang -MD -MF - %s -fsyntax-only -fms-compatibility -I %t.dir/include
| FileCheck -check-prefix=CHECK-ONE %s
+// CHECK-ONE: foo/bar.h
+// CHECK-ONE-NOT: foo\bar.h
+// CHECK-ONE-NOT: foo\\bar.h
+
+// RUN: %clang -MD -MF - %s -fsyntax-only -fno-ms-compatibility -I
%t.dir/include | FileCheck -check-prefix=CHECK-TWO %s
+// CHECK-TWO: foo\bar.h
+// CHECK-TWO-NOT: foo/bar.h
+// CHECK-TWO-NOT: foo\\bar.h
+
+#include "foo\bar.h"
``````````
</details>
https://github.com/llvm/llvm-project/pull/160903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
