[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342139: Print correctly dependency paths on Windows 
(authored by xbolva00, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51847

Files:
  lib/Frontend/DependencyFile.cpp
  test/Frontend/dependency-gen-escaping.c
  test/Frontend/dependency-gen.c
  test/Modules/relative-dep-gen.cpp

Index: test/Frontend/dependency-gen-escaping.c
===
--- test/Frontend/dependency-gen-escaping.c
+++ test/Frontend/dependency-gen-escaping.c
@@ -21,7 +21,7 @@
 // Backslash followed by # or space should escape both characters, because
 // that's what GNU Make wants.  GCC does the right thing with space, but not
 // #, so Clang does too. (There should be 3 backslashes before the #.)
-// SEP2F: a\b\\#c\\\ d.h
+// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
 // With -fms-compatibility, Backslashes in #include are treated as path separators.
 // Backslashes are given in the emission for special characters, like 0x20 or 0x23.
 // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -4,19 +4,19 @@
 // RUN: echo > %t.dir/a/b/x.h
 // RUN: cd %t.dir
 // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b | FileCheck -check-prefix=CHECK-ONE %s
-// CHECK-ONE: {{ }}a/b{{[/\\]}}x.h
+// CHECK-ONE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 
 // PR8974 (-include flag)
 // RUN: %clang -MD -MF - %s -fsyntax-only -include a/b/x.h -DINCLUDE_FLAG_TEST | FileCheck -check-prefix=CHECK-TWO %s
-// CHECK-TWO: {{ }}a/b/x.h
+// CHECK-TWO: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 
 // rdar://problem/9734352 (paths involving ".")
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./a/b | FileCheck -check-prefix=CHECK-THREE %s
-// CHECK-THREE: {{ }}a/b{{[/\\]}}x.h
+// CHECK-THREE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 // RUN: %clang -MD -MF - %s -fsyntax-only -I .//./a/b/ | FileCheck -check-prefix=CHECK-FOUR %s
-// CHECK-FOUR: {{ }}a/b{{[/\\]}}x.h
+// CHECK-FOUR: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b/. | FileCheck -check-prefix=CHECK-FIVE %s
-// CHECK-FIVE: {{ }}a/b/.{{[/\\]}}x.h
+// CHECK-FIVE: {{ }}a{{[/\\]}}b{{[/\\]}}.{{[/\\]}}x.h
 // RUN: cd a/b
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s
 // CHECK-SIX: {{ }}x.h
Index: test/Modules/relative-dep-gen.cpp
===
--- test/Modules/relative-dep-gen.cpp
+++ test/Modules/relative-dep-gen.cpp
@@ -30,9 +30,9 @@
 #include "Inputs/relative-dep-gen-1.h"
 
 // CHECK-BUILD: mod.pcm:
-// CHECK-BUILD:   {{[ \t]}}Inputs/relative-dep-gen{{(-cwd)?}}.modulemap
-// CHECK-BUILD:   {{[ \t]}}Inputs/relative-dep-gen-1.h
-// CHECK-BUILD:   {{[ \t]}}Inputs/relative-dep-gen-2.h
+// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen{{(-cwd)?}}.modulemap
+// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-1.h
+// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-2.h
 // CHECK-USE: use.o:
 // CHECK-USE-DAG:   {{[ \t]}}relative-dep-gen.cpp
 // CHECK-EXPLICIT-DAG:   mod.pcm
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -386,28 +386,32 @@
 /// for Windows file-naming info.
 static void PrintFilename(raw_ostream &OS, StringRef Filename,
   DependencyOutputFormat OutputFormat) {
+  // Convert filename to platform native path
+  llvm::SmallString<256> NativePath;
+  llvm::sys::path::native(Filename.str(), NativePath);
+
   if (OutputFormat == DependencyOutputFormat::NMake) {
 // Add quotes if needed. These are the characters listed as "special" to
 // NMake, that are legal in a Windows filespec, and that could cause
 // misinterpretation of the dependency string.
-if (Filename.find_first_of(" #${}^!") != StringRef::npos)
-  OS << '\"' << Filename << '\"';
+if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
+  OS << '\"' << NativePath << '\"';
 else
-  OS << Filename;
+  OS << NativePath;
 return;
   }
   assert(OutputFormat == DependencyOutputFormat::Make);
-  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
-if (Filename[i] == '#') // Handle '#' the broken gcc way.
+  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
+if (NativePath[i] == '#') // Handle '#' the broken gcc way.
   OS << '\\';
-else if (Filename[i] == ' ') { // Handle space correctly.
+else if (NativePath[i] == ' ') { // Handle space correctly.
   OS << '\\';
   unsigned j = i;
-  while (j > 0 && Filename[--j] == '\\')
+  while (j > 0 && NativePath[--j] == '\\')
 

[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 165204.
xbolva00 added a comment.

Adjusted regexes in two more tests


https://reviews.llvm.org/D51847

Files:
  lib/Frontend/DependencyFile.cpp
  test/Frontend/dependency-gen-escaping.c
  test/Frontend/dependency-gen.c
  test/Modules/relative-dep-gen.cpp

Index: test/Modules/relative-dep-gen.cpp
===
--- test/Modules/relative-dep-gen.cpp
+++ test/Modules/relative-dep-gen.cpp
@@ -30,9 +30,9 @@
 #include "Inputs/relative-dep-gen-1.h"
 
 // CHECK-BUILD: mod.pcm:
-// CHECK-BUILD:   {{[ \t]}}Inputs/relative-dep-gen{{(-cwd)?}}.modulemap
-// CHECK-BUILD:   {{[ \t]}}Inputs/relative-dep-gen-1.h
-// CHECK-BUILD:   {{[ \t]}}Inputs/relative-dep-gen-2.h
+// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen{{(-cwd)?}}.modulemap
+// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-1.h
+// CHECK-BUILD:   {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-2.h
 // CHECK-USE: use.o:
 // CHECK-USE-DAG:   {{[ \t]}}relative-dep-gen.cpp
 // CHECK-EXPLICIT-DAG:   mod.pcm
Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -4,19 +4,19 @@
 // RUN: echo > %t.dir/a/b/x.h
 // RUN: cd %t.dir
 // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b | FileCheck -check-prefix=CHECK-ONE %s
-// CHECK-ONE: {{ }}a/b{{[/\\]}}x.h
+// CHECK-ONE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 
 // PR8974 (-include flag)
 // RUN: %clang -MD -MF - %s -fsyntax-only -include a/b/x.h -DINCLUDE_FLAG_TEST | FileCheck -check-prefix=CHECK-TWO %s
-// CHECK-TWO: {{ }}a/b/x.h
+// CHECK-TWO: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 
 // rdar://problem/9734352 (paths involving ".")
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./a/b | FileCheck -check-prefix=CHECK-THREE %s
-// CHECK-THREE: {{ }}a/b{{[/\\]}}x.h
+// CHECK-THREE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 // RUN: %clang -MD -MF - %s -fsyntax-only -I .//./a/b/ | FileCheck -check-prefix=CHECK-FOUR %s
-// CHECK-FOUR: {{ }}a/b{{[/\\]}}x.h
+// CHECK-FOUR: {{ }}a{{[/\\]}}b{{[/\\]}}x.h
 // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b/. | FileCheck -check-prefix=CHECK-FIVE %s
-// CHECK-FIVE: {{ }}a/b/.{{[/\\]}}x.h
+// CHECK-FIVE: {{ }}a{{[/\\]}}b{{[/\\]}}.{{[/\\]}}x.h
 // RUN: cd a/b
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s
 // CHECK-SIX: {{ }}x.h
Index: test/Frontend/dependency-gen-escaping.c
===
--- test/Frontend/dependency-gen-escaping.c
+++ test/Frontend/dependency-gen-escaping.c
@@ -21,7 +21,7 @@
 // Backslash followed by # or space should escape both characters, because
 // that's what GNU Make wants.  GCC does the right thing with space, but not
 // #, so Clang does too. (There should be 3 backslashes before the #.)
-// SEP2F: a\b\\#c\\\ d.h
+// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
 // With -fms-compatibility, Backslashes in #include are treated as path separators.
 // Backslashes are given in the emission for special characters, like 0x20 or 0x23.
 // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -386,28 +386,32 @@
 /// for Windows file-naming info.
 static void PrintFilename(raw_ostream &OS, StringRef Filename,
   DependencyOutputFormat OutputFormat) {
+  // Convert filename to platform native path
+  llvm::SmallString<256> NativePath;
+  llvm::sys::path::native(Filename.str(), NativePath);
+
   if (OutputFormat == DependencyOutputFormat::NMake) {
 // Add quotes if needed. These are the characters listed as "special" to
 // NMake, that are legal in a Windows filespec, and that could cause
 // misinterpretation of the dependency string.
-if (Filename.find_first_of(" #${}^!") != StringRef::npos)
-  OS << '\"' << Filename << '\"';
+if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
+  OS << '\"' << NativePath << '\"';
 else
-  OS << Filename;
+  OS << NativePath;
 return;
   }
   assert(OutputFormat == DependencyOutputFormat::Make);
-  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
-if (Filename[i] == '#') // Handle '#' the broken gcc way.
+  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
+if (NativePath[i] == '#') // Handle '#' the broken gcc way.
   OS << '\\';
-else if (Filename[i] == ' ') { // Handle space correctly.
+else if (NativePath[i] == ' ') { // Handle space correctly.
   OS << '\\';
   unsigned j = i;
-  while (j > 0 && Filename[--j] == '\\')
+  while (j > 0 && NativePath[--j] == '\\')
 OS << '\\';
-} else if (Filename[i] == '$') // $ is escaped by $$.
+} else if (NativePath[i] == '$') // $ is escaped by $$.
   OS << '$';
-OS << Filename[i];
+OS 

[PATCH] D51847: Print correctly dependency paths on Windows

[Mailing List "cfe-commits" via Phabricator via cfe-commits]

cfe-commits added a comment.

Lgtm

- F7184192: msg-6991-166.txt 


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via cfe-commits]

Lgtm
On Wed, Sep 12, 2018 at 3:23 AM Dávid Bolvanský via Phabricator <
revi...@reviews.llvm.org> wrote:

> xbolva00 updated this revision to Diff 165052.
> xbolva00 added a comment.
>
> - Fixed failing test
>
>
> https://reviews.llvm.org/D51847
>
> Files:
>   lib/Frontend/DependencyFile.cpp
>   test/Frontend/dependency-gen-escaping.c
>
>
> Index: test/Frontend/dependency-gen-escaping.c
> ===
> --- test/Frontend/dependency-gen-escaping.c
> +++ test/Frontend/dependency-gen-escaping.c
> @@ -21,7 +21,7 @@
>  // Backslash followed by # or space should escape both characters, because
>  // that's what GNU Make wants.  GCC does the right thing with space, but
> not
>  // #, so Clang does too. (There should be 3 backslashes before the #.)
> -// SEP2F: a\b\\#c\\\ d.h
> +// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
>  // With -fms-compatibility, Backslashes in #include are treated as path
> separators.
>  // Backslashes are given in the emission for special characters, like
> 0x20 or 0x23.
>  // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
> Index: lib/Frontend/DependencyFile.cpp
> ===
> --- lib/Frontend/DependencyFile.cpp
> +++ lib/Frontend/DependencyFile.cpp
> @@ -386,28 +386,32 @@
>  /// for Windows file-naming info.
>  static void PrintFilename(raw_ostream &OS, StringRef Filename,
>DependencyOutputFormat OutputFormat) {
> +  // Convert filename to platform native path
> +  llvm::SmallString<256> NativePath;
> +  llvm::sys::path::native(Filename.str(), NativePath);
> +
>if (OutputFormat == DependencyOutputFormat::NMake) {
>  // Add quotes if needed. These are the characters listed as "special"
> to
>  // NMake, that are legal in a Windows filespec, and that could cause
>  // misinterpretation of the dependency string.
> -if (Filename.find_first_of(" #${}^!") != StringRef::npos)
> -  OS << '\"' << Filename << '\"';
> +if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
> +  OS << '\"' << NativePath << '\"';
>  else
> -  OS << Filename;
> +  OS << NativePath;
>  return;
>}
>assert(OutputFormat == DependencyOutputFormat::Make);
> -  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
> -if (Filename[i] == '#') // Handle '#' the broken gcc way.
> +  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
> +if (NativePath[i] == '#') // Handle '#' the broken gcc way.
>OS << '\\';
> -else if (Filename[i] == ' ') { // Handle space correctly.
> +else if (NativePath[i] == ' ') { // Handle space correctly.
>OS << '\\';
>unsigned j = i;
> -  while (j > 0 && Filename[--j] == '\\')
> +  while (j > 0 && NativePath[--j] == '\\')
>  OS << '\\';
> -} else if (Filename[i] == '$') // $ is escaped by $$.
> +} else if (NativePath[i] == '$') // $ is escaped by $$.
>OS << '$';
> -OS << Filename[i];
> +OS << NativePath[i];
>}
>  }
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 165052.
xbolva00 added a comment.

- Fixed failing test


https://reviews.llvm.org/D51847

Files:
  lib/Frontend/DependencyFile.cpp
  test/Frontend/dependency-gen-escaping.c


Index: test/Frontend/dependency-gen-escaping.c
===
--- test/Frontend/dependency-gen-escaping.c
+++ test/Frontend/dependency-gen-escaping.c
@@ -21,7 +21,7 @@
 // Backslash followed by # or space should escape both characters, because
 // that's what GNU Make wants.  GCC does the right thing with space, but not
 // #, so Clang does too. (There should be 3 backslashes before the #.)
-// SEP2F: a\b\\#c\\\ d.h
+// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
 // With -fms-compatibility, Backslashes in #include are treated as path 
separators.
 // Backslashes are given in the emission for special characters, like 0x20 or 
0x23.
 // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -386,28 +386,32 @@
 /// for Windows file-naming info.
 static void PrintFilename(raw_ostream &OS, StringRef Filename,
   DependencyOutputFormat OutputFormat) {
+  // Convert filename to platform native path
+  llvm::SmallString<256> NativePath;
+  llvm::sys::path::native(Filename.str(), NativePath);
+
   if (OutputFormat == DependencyOutputFormat::NMake) {
 // Add quotes if needed. These are the characters listed as "special" to
 // NMake, that are legal in a Windows filespec, and that could cause
 // misinterpretation of the dependency string.
-if (Filename.find_first_of(" #${}^!") != StringRef::npos)
-  OS << '\"' << Filename << '\"';
+if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
+  OS << '\"' << NativePath << '\"';
 else
-  OS << Filename;
+  OS << NativePath;
 return;
   }
   assert(OutputFormat == DependencyOutputFormat::Make);
-  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
-if (Filename[i] == '#') // Handle '#' the broken gcc way.
+  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
+if (NativePath[i] == '#') // Handle '#' the broken gcc way.
   OS << '\\';
-else if (Filename[i] == ' ') { // Handle space correctly.
+else if (NativePath[i] == ' ') { // Handle space correctly.
   OS << '\\';
   unsigned j = i;
-  while (j > 0 && Filename[--j] == '\\')
+  while (j > 0 && NativePath[--j] == '\\')
 OS << '\\';
-} else if (Filename[i] == '$') // $ is escaped by $$.
+} else if (NativePath[i] == '$') // $ is escaped by $$.
   OS << '$';
-OS << Filename[i];
+OS << NativePath[i];
   }
 }
 


Index: test/Frontend/dependency-gen-escaping.c
===
--- test/Frontend/dependency-gen-escaping.c
+++ test/Frontend/dependency-gen-escaping.c
@@ -21,7 +21,7 @@
 // Backslash followed by # or space should escape both characters, because
 // that's what GNU Make wants.  GCC does the right thing with space, but not
 // #, so Clang does too. (There should be 3 backslashes before the #.)
-// SEP2F: a\b\\#c\\\ d.h
+// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
 // With -fms-compatibility, Backslashes in #include are treated as path separators.
 // Backslashes are given in the emission for special characters, like 0x20 or 0x23.
 // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -386,28 +386,32 @@
 /// for Windows file-naming info.
 static void PrintFilename(raw_ostream &OS, StringRef Filename,
   DependencyOutputFormat OutputFormat) {
+  // Convert filename to platform native path
+  llvm::SmallString<256> NativePath;
+  llvm::sys::path::native(Filename.str(), NativePath);
+
   if (OutputFormat == DependencyOutputFormat::NMake) {
 // Add quotes if needed. These are the characters listed as "special" to
 // NMake, that are legal in a Windows filespec, and that could cause
 // misinterpretation of the dependency string.
-if (Filename.find_first_of(" #${}^!") != StringRef::npos)
-  OS << '\"' << Filename << '\"';
+if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
+  OS << '\"' << NativePath << '\"';
 else
-  OS << Filename;
+  OS << NativePath;
 return;
   }
   assert(OutputFormat == DependencyOutputFormat::Make);
-  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
-if (Filename[i] == '#') // Handle '#' the broken gcc way.
+  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
+if (NativePath[i] == '#') // Handle '#' the broken gcc way.
   OS << '\\';
-else if (Filename[i] == ' ') { // Handle space c

[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 165012.
xbolva00 added a comment.

Convert paths to platform native paths


https://reviews.llvm.org/D51847

Files:
  lib/Frontend/DependencyFile.cpp


Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -386,28 +386,32 @@
 /// for Windows file-naming info.
 static void PrintFilename(raw_ostream &OS, StringRef Filename,
   DependencyOutputFormat OutputFormat) {
+  // Convert filename to platform native path
+  llvm::SmallString<256> NativePath;
+  llvm::sys::path::native(Filename.str(), NativePath);
+
   if (OutputFormat == DependencyOutputFormat::NMake) {
 // Add quotes if needed. These are the characters listed as "special" to
 // NMake, that are legal in a Windows filespec, and that could cause
 // misinterpretation of the dependency string.
-if (Filename.find_first_of(" #${}^!") != StringRef::npos)
-  OS << '\"' << Filename << '\"';
+if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
+  OS << '\"' << NativePath << '\"';
 else
-  OS << Filename;
+  OS << NativePath;
 return;
   }
   assert(OutputFormat == DependencyOutputFormat::Make);
-  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
-if (Filename[i] == '#') // Handle '#' the broken gcc way.
+  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
+if (NativePath[i] == '#') // Handle '#' the broken gcc way.
   OS << '\\';
-else if (Filename[i] == ' ') { // Handle space correctly.
+else if (NativePath[i] == ' ') { // Handle space correctly.
   OS << '\\';
   unsigned j = i;
-  while (j > 0 && Filename[--j] == '\\')
+  while (j > 0 && NativePath[--j] == '\\')
 OS << '\\';
-} else if (Filename[i] == '$') // $ is escaped by $$.
+} else if (NativePath[i] == '$') // $ is escaped by $$.
   OS << '$';
-OS << Filename[i];
+OS << NativePath[i];
   }
 }
 


Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -386,28 +386,32 @@
 /// for Windows file-naming info.
 static void PrintFilename(raw_ostream &OS, StringRef Filename,
   DependencyOutputFormat OutputFormat) {
+  // Convert filename to platform native path
+  llvm::SmallString<256> NativePath;
+  llvm::sys::path::native(Filename.str(), NativePath);
+
   if (OutputFormat == DependencyOutputFormat::NMake) {
 // Add quotes if needed. These are the characters listed as "special" to
 // NMake, that are legal in a Windows filespec, and that could cause
 // misinterpretation of the dependency string.
-if (Filename.find_first_of(" #${}^!") != StringRef::npos)
-  OS << '\"' << Filename << '\"';
+if (NativePath.find_first_of(" #${}^!") != StringRef::npos)
+  OS << '\"' << NativePath << '\"';
 else
-  OS << Filename;
+  OS << NativePath;
 return;
   }
   assert(OutputFormat == DependencyOutputFormat::Make);
-  for (unsigned i = 0, e = Filename.size(); i != e; ++i) {
-if (Filename[i] == '#') // Handle '#' the broken gcc way.
+  for (unsigned i = 0, e = NativePath.size(); i != e; ++i) {
+if (NativePath[i] == '#') // Handle '#' the broken gcc way.
   OS << '\\';
-else if (Filename[i] == ' ') { // Handle space correctly.
+else if (NativePath[i] == ' ') { // Handle space correctly.
   OS << '\\';
   unsigned j = i;
-  while (j > 0 && Filename[--j] == '\\')
+  while (j > 0 && NativePath[--j] == '\\')
 OS << '\\';
-} else if (Filename[i] == '$') // $ is escaped by $$.
+} else if (NativePath[i] == '$') // $ is escaped by $$.
   OS << '$';
-OS << Filename[i];
+OS << NativePath[i];
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

clang-cl in general tries to do the thing that makes native Windows developers 
the most comfortable, while gcc in general tries to do the thing that makes 
Unix developers most comfortable.  So I think we should probably use \ here.  
If anyone complains we can revisit.


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In https://reviews.llvm.org/D51847#1230766, @zturner wrote:

> Alright I see it.  The reason I'm asking is because after your change you're 
> now printing `main.o: main.c ../include/lib/test.h`.  But I wonder if you 
> should instead be printing `main.o: main.c ..\include\lib\test.h`.  It seems 
> like paths that we show to the user should be in the native path style.


Possibly, or should we follow GGC behavior here? GCC prints  `main.o: main.c 
../include/lib/test.h`. But conversion to native paths sounds fine for me as 
well.


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

Alright I see it.  The reason I'm asking is because after your change you're 
now printing `main.o: main.c ../include/lib/test.h`.  But I wonder if you 
should instead be printing `main.o: main.c ..\include\lib\test.h`.  It seems 
like paths that we show to the user should be in the native path style.




Comment at: lib/Frontend/DependencyFile.cpp:389
   DependencyOutputFormat OutputFormat) {
+  std::string &NativePath = llvm::sys::path::convert_to_slash(Filename);
   if (OutputFormat == DependencyOutputFormat::NMake) {

This looks like a dangling reference -- undefined behavior.


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Please see PR38877.


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

I mean in practice. What command do i run to hit this and what will the
output look like? Can you paste some terminal output showing the command
and output?


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via cfe-commits]

I mean in practice. What command do i run to hit this and what will the
output look like? Can you paste some terminal output showing the command
and output?
On Tue, Sep 11, 2018 at 12:55 AM Dávid Bolvanský via Phabricator <
revi...@reviews.llvm.org> wrote:

> xbolva00 added a comment.
>
> In https://reviews.llvm.org/D51847#1228927, @zturner wrote:
>
> > What prints this? How do you exercise this codepath?
>
>
> DFGImpl::OutputDependencyFile() -> for (StringRef File : Files)
>
>
> https://reviews.llvm.org/D51847
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In https://reviews.llvm.org/D51847#1228927, @zturner wrote:

> What prints this? How do you exercise this codepath?


DFGImpl::OutputDependencyFile() -> for (StringRef File : Files)


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a subscriber: xbolva00.
zturner added a comment.

What prints this? How do you exercise this codepath?


https://reviews.llvm.org/D51847



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits