[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-27 Thread Paul Robinson via Phabricator via lldb-commits
probinson added a comment.

In D49466#1761156 , @MaskRay wrote:

> The ugly path separator pattern `{{(/|)}}` appears in 60+ tests. Can we 
> teach clang and other tools to
>
> 1. accept both `/` and `\` input


In general they do, AFAIK, although it's not feasible in cases where `/` is the 
character that introduces an option, which is common on standard Windows 
utilities.

> 2. but only output `/`
> 
>   on Windows?

This is often actually incorrect for Windows.

> We can probably remove llvm::sys::path::Style::{windows,posix,native} from 
> include/llvm/Support/Path.h and only keep the posix form.

It's highly unlikely that will be correct for all cases, and certainly will not 
match users' expectations.  Debug info, for example, will want normal Windows 
syntax file paths with `\`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Nico Weber via Phabricator via lldb-commits
thakis added a comment.

There's still one failing test on Windows after the fix attempt: 
http://45.33.8.238/win/3052/step_6.txt

Please take a look and revert if it's not an easy fix. (And please watch bots 
after committing stuff.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

The ugly path separator pattern `{{(/|)}}` appears in 60+ tests. Can we 
teach clang and other tools to

1. accept both `/` and `\` input
2. but only output `/`

on Windows? We can probably remove 
`llvm::sys::path::Style::{windows,posix,native}` from 
`include/llvm/Support/Path.h` and only keep the posix form.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

Does this work on Windows?

  --- i/clang/test/Preprocessor/file_test.c
  +++ w/clang/test/Preprocessor/file_test.c
  @@ -1,8 +1,7 @@
  -// XFAIL: system-windows
   // RUN: %clang -E -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | 
FileCheck %s
   // RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | 
FileCheck %s
   // RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH=empty -c -o - %s | 
FileCheck %s -check-prefix CHECK-EVIL
  -// RUN: %clang -E -fmacro-prefix-map=%p/= -c -o - %s | FileCheck %s 
--check-prefix CHECK-REMOVE
  +// RUN: %clang -E -fmacro-prefix-map=%/p/= -c -o - %/s | FileCheck %s 
--check-prefix CHECK-REMOVE

`startswith` is not ideal because `/t` will match `/tmp`. However, gcc appears 
to use something similar to `startswith`, see:

  % gcc -c -g -fdebug-prefix-map=/t=x /tmp/c/a.c -o /tmp/c/a.o
  % llvm-dwarfdump /tmp/c/a.o | grep -m 1 DW_AT_name
DW_AT_name("xmp/c/a.c")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 231142.
MaskRay added a comment.

Minimize diff in Options.td
Properly rebase remapDIPath on top of D69213 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl ,
+bool replace_path_prefix(SmallVectorImpl ,
  const StringRef , const StringRef ,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine , SmallVectorImpl , Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -152,18 +152,33 @@
 ///
 /// @code
 ///   

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 231140.
MaskRay added a comment.

Add back remapDIPath that was unintentionally deleted by D69213 
, caught by a test.

Small adjustment of the code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl ,
+bool replace_path_prefix(SmallVectorImpl ,
  const StringRef , const StringRef ,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine , SmallVectorImpl , Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ 

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

In D49466#1761044 , @thakis wrote:

> There's still one failing test on Windows after the fix attempt: 
> http://45.33.8.238/win/3052/step_6.txt
>
> Please take a look and revert if it's not an easy fix. (And please watch bots 
> after committing stuff.)


XFAIL'ed `clang/test/Preprocessor/file_test.c`. I didn't receive an email from 
http://45.33.8.238/win/3052/step_6.txt not sure if that was because the author 
email was @dankm's, not mine. If there is a console. please tell me :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Dan McGregor via Phabricator via lldb-commits
dankm added a comment.

In D49466#1760860 , @MaskRay wrote:

> Add back remapDIPath that was unintentionally deleted by D69213 
> , caught by a test.
>
> Small adjustment of the code


Right. Can't believe I missed that. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c92cdff7225: Initial implementation of -fmacro-prefix-map 
and -ffile-prefix-map (authored by dankm, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl ,
+bool replace_path_prefix(SmallVectorImpl ,
  const StringRef , const StringRef ,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine , SmallVectorImpl , Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ 

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Dan McGregor via Phabricator via lldb-commits
dankm added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

In D49466#1760765 , @dankm wrote:

> Ping?


The tests need fixing... I can commit it. Now that we've migrated to the llvm 
monorepo, the git commit message can retain the author info properly...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via lldb-commits
dankm added a comment.

Whoops. There are extra changes here I didn't mean to submit :/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via lldb-commits
dankm updated this revision to Diff 229835.
dankm added a comment.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

- Address feedback from @Lekensteyn


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  lld/Common/CMakeLists.txt
  lldb/source/CMakeLists.txt
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl ,
+bool replace_path_prefix(SmallVectorImpl ,
  const StringRef , const StringRef ,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine , SmallVectorImpl , Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ 

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via lldb-commits
dankm added a comment.

In D49466#1681534 , @phosek wrote:

> @dankm is it OK if we take over this change to push it forward?


At this point sure. Unless it's accepted as-is now, then I don't have a commit 
bit to finish it off.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466



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


[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via lldb-commits
dankm updated this revision to Diff 229837.
dankm added a comment.

- Address feedback from @Lekensteyn


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49466/new/

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl ,
+bool replace_path_prefix(SmallVectorImpl ,
  const StringRef , const StringRef ,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine , SmallVectorImpl , Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -152,18 +152,33 @@
 ///
 /// @code
 ///   /foo, /old, /new => /foo
+///   /old, /old, /new => /new
+///   /old,