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

[Paul Robinson via Phabricator via cfe-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



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

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

[Fangrui Song via Phabricator via cfe-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



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

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

[Fangrui Song via Phabricator via cfe-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



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

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

[Fangrui Song via Phabricator via cfe-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



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

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

[Nico Weber via Phabricator via cfe-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



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

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

[Dan McGregor via Phabricator via cfe-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



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

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

[Fangrui Song via Phabricator via cfe-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
 ///   

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

[Fangrui Song via Phabricator via cfe-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
@@ 

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

[Fangrui Song via Phabricator via cfe-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
@@ 

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

[Fangrui Song via Phabricator via cfe-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



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

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

[Dan McGregor via Phabricator via cfe-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



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

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

[Dan McGregor via Phabricator via cfe-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



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

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

[Dan McGregor via Phabricator via cfe-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, 

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

[Dan McGregor via Phabricator via cfe-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



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

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

[Dan McGregor via Phabricator via cfe-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
+++ 

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked an inline comment as done.
dankm added inline comments.



Comment at: clang/test/Driver/debug-prefix-map.c:8
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s 
-check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s 
-check-prefix CHECK-MACRO-SIMPLE
+

Lekensteyn wrote:
> What about combining these two tests? The command is the same, maybe you 
> could have a new `-check-prefix` to reduce the number of invocations? 
> Likewise for the cases below.
The tests will need more thinking, you're probably right, but I don't have much 
time to work on this at the moment. How do you feel about addressing this in 
the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 2 inline comments as done.
dankm added inline comments.



Comment at: llvm/include/llvm/Support/Path.h:172
+/// @param style The path separator style
+/// @param strict Strict prefix path checking
+/// @result true if \a Path begins with OldPrefix

Lekensteyn wrote:
> "strict checking" is ambiguous on its own. What about something like:
> 
> If strict is true, a directory separator following \a OldPrefix will also 
> be stripped. Otherwise, directory separators will only be matched and 
> stripped when present in \a OldPrefix.
> 
> Or whatever semantics you would like to assign to "strict mode".
Thanks. I like your wording, and I've used it in the incoming patch.



Comment at: llvm/include/llvm/Support/Path.h:181
+  return replace_path_prefix(Path, OldPrefix, NewPrefix, style, strict);
+}
 

Lekensteyn wrote:
> Why have a variant with the parameters swapped, is it common in LLVM to have 
> such convenience wrappers?
> 
> Why not require callers to pass `Style::native` whenever they want to modify 
> "strict"?
Works for me. I did that to make my call sites somewhat more readable. It'll be 
changed too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

It'd be nice to see this land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

@dankm do you still plan to work on this? We would really like to see this 
landed and we could help if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Thanks for picking this up again. I've left some nitpicks below in a quick 
review.

The "strict" parameter is not precisely defined, if that is fixed I think this 
would be ready for merge.




Comment at: clang/test/Driver/debug-prefix-map.c:8
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s 
-check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s 
-check-prefix CHECK-MACRO-SIMPLE
+

What about combining these two tests? The command is the same, maybe you could 
have a new `-check-prefix` to reduce the number of invocations? Likewise for 
the cases below.



Comment at: llvm/include/llvm/Support/Path.h:172
+/// @param style The path separator style
+/// @param strict Strict prefix path checking
+/// @result true if \a Path begins with OldPrefix

"strict checking" is ambiguous on its own. What about something like:

If strict is true, a directory separator following \a OldPrefix will also 
be stripped. Otherwise, directory separators will only be matched and stripped 
when present in \a OldPrefix.

Or whatever semantics you would like to assign to "strict mode".



Comment at: llvm/include/llvm/Support/Path.h:181
+  return replace_path_prefix(Path, OldPrefix, NewPrefix, style, strict);
+}
 

Why have a variant with the parameters swapped, is it common in LLVM to have 
such convenience wrappers?

Why not require callers to pass `Style::native` whenever they want to modify 
"strict"?



Comment at: llvm/lib/Support/Path.cpp:512
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+

this condition is duplicated above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 212723.
dankm added a comment.
Herald added subscribers: llvm-commits, ormris, hiraditya.
Herald added a project: LLVM.

Latest changes. I've been sitting on these for months, so I don't remember all 
that changed. The path remapping contract changed somewhat, and it's now based 
on the git monorepo.


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
@@ -1219,7 +1219,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("");
@@ -1239,6 +1241,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,53 @@
   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;
+
+  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 

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

[Dan McGregor via Phabricator via cfe-commits]

dankm added a comment.

In D49466#1586140 , @manojgupta wrote:

> @dankm are you still working on this patch?


Yes, I've been afk for a bit due to family circumstances, but I just uploaded 
more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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

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

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

@dankm are you still working on this patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Hi @dankm, any progress on this feature? The proposed branch off date for Clang 
9.0.0 is 18 July 2019: 
https://lists.llvm.org/pipermail/cfe-dev/2019-June/062628.html


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Khem Raj via Phabricator via cfe-commits]

raj.khem added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:476
   for (const auto  : DebugPrefixMap)
-if (Path.startswith(Entry.first))
-  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
-  return Path.str();
+if (llvm::sys::path::replace_path_prefix(p, Entry.first, Entry.second))
+  break;

raj.khem wrote:
> looking at llvm/lib/Support/Path.cpp replace_path_prefix() returns void but 
> here inside if() it will expect a bool return value 
nm I guess I needed to look into https://reviews.llvm.org/D56769 as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Khem Raj via Phabricator via cfe-commits]

raj.khem added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:476
   for (const auto  : DebugPrefixMap)
-if (Path.startswith(Entry.first))
-  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
-  return Path.str();
+if (llvm::sys::path::replace_path_prefix(p, Entry.first, Entry.second))
+  break;

looking at llvm/lib/Support/Path.cpp replace_path_prefix() returns void but 
here inside if() it will expect a bool return value 


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 182121.
dankm added a comment.

Move trailing path separator stripping back to Clang.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/debug-prefix-map.c
===
--- test/Driver/debug-prefix-map.c
+++ test/Driver/debug-prefix-map.c
@@ -1,9 +1,28 @@
-// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-INVALID
-// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-SIMPLE
-// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COMPLEX
-// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-EMPTY
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
 
-// CHECK-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
-// CHECK-SIMPLE: fdebug-prefix-map=old=new
-// CHECK-COMPLEX: fdebug-prefix-map=old=n=ew
-// CHECK-EMPTY: fdebug-prefix-map=old=
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-DEBUG-EMPTY: 

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked an inline comment as done.
dankm added a comment.

Sure, I'll (eventually) make a separate review.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 182047.
dankm added a comment.

Undo accidental change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/debug-prefix-map.c
===
--- test/Driver/debug-prefix-map.c
+++ test/Driver/debug-prefix-map.c
@@ -1,9 +1,28 @@
-// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-INVALID
-// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-SIMPLE
-// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COMPLEX
-// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-EMPTY
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
 
-// CHECK-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
-// CHECK-SIMPLE: fdebug-prefix-map=old=new
-// CHECK-COMPLEX: fdebug-prefix-map=old=n=ew
-// CHECK-EMPTY: fdebug-prefix-map=old=
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-DEBUG-EMPTY: fdebug-prefix-map=old=
+// 

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:607
   llvm::DIFile *CUFile = DBuilder.createFile(
-  remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
+  MainFileName, remapDIPath(getCurrentDirname()), CSInfo,
   getSource(SM, SM.getMainFileID()));

dankm wrote:
> Lekensteyn wrote:
> > Any reason for dropping `remapDIPath` here? Wouldn't this result in the 
> > full path being included even when using:
> > 
> > clang -fdebug-prefix-map=/full/path/= /full/path/source.c
> Whoops. That probably shouldn't have been included this round. This is a 
> bugfix. MainFileName is already remapped from earlier in this function, this 
> keeps it from remapping twice if you have an empty old prefix.
The remapping was done here:
```c
  std::string MainFileDir;
  if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {
MainFileDir = remapDIPath(MainFile->getDir()->getName());
```

(Observation: the declaration could probably be moved inside the `if` block 
since it is not used outside.)

What about the second case though? For example, assume `/tmp/testdir/mytest.ii`:
```
# 1 "/tmp/mytest.c"
# 1 ""
# 1 ""
# 31 ""
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "" 2
# 1 "/tmp/mytest.c"
int main(int argc, const char *argv[])
{
return 0;
}
```

What happens if you now compile with `clang -fdebug-prefix-map=/tmp/=/bla/ 
/tmp/testdir/mytest.ii` from `/tmp/testdir`?

Unless this affects the current patch, consider moving it to a separate change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 182037.
dankm added a comment.

Update style.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/debug-prefix-map.c
===
--- test/Driver/debug-prefix-map.c
+++ test/Driver/debug-prefix-map.c
@@ -1,9 +1,28 @@
-// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-INVALID
-// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-SIMPLE
-// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COMPLEX
-// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-EMPTY
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
 
-// CHECK-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
-// CHECK-SIMPLE: fdebug-prefix-map=old=new
-// CHECK-COMPLEX: fdebug-prefix-map=old=n=ew
-// CHECK-EMPTY: fdebug-prefix-map=old=
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-DEBUG-EMPTY: fdebug-prefix-map=old=
+// CHECK-MACRO-EMPTY: 

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked an inline comment as done.
dankm added a comment.

  I'll update the style nit, and spend some non-tired time on the string 
remapping. Thanks




Comment at: lib/CodeGen/CGDebugInfo.cpp:607
   llvm::DIFile *CUFile = DBuilder.createFile(
-  remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
+  MainFileName, remapDIPath(getCurrentDirname()), CSInfo,
   getSource(SM, SM.getMainFileID()));

Lekensteyn wrote:
> Any reason for dropping `remapDIPath` here? Wouldn't this result in the full 
> path being included even when using:
> 
> clang -fdebug-prefix-map=/full/path/= /full/path/source.c
Whoops. That probably shouldn't have been included this round. This is a 
bugfix. MainFileName is already remapped from earlier in this function, this 
keeps it from remapping twice if you have an empty old prefix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Changes still look reasonable, but the preceding path 
(https://reviews.llvm.org/D56769) needs some work.




Comment at: lib/CodeGen/CGDebugInfo.cpp:607
   llvm::DIFile *CUFile = DBuilder.createFile(
-  remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
+  MainFileName, remapDIPath(getCurrentDirname()), CSInfo,
   getSource(SM, SM.getMainFileID()));

Any reason for dropping `remapDIPath` here? Wouldn't this result in the full 
path being included even when using:

clang -fdebug-prefix-map=/full/path/= /full/path/source.c



Comment at: lib/Lex/PPMacroExpansion.cpp:1466
+  for (const auto  : MacroPrefixMap)
+if(llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
+  break;

Style: space between `if` and `(`


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 181964.
dankm marked an inline comment as done.
dankm added a comment.

Enforce path mapping. This requires LLVM review D56769 
.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/debug-prefix-map.c
===
--- test/Driver/debug-prefix-map.c
+++ test/Driver/debug-prefix-map.c
@@ -1,9 +1,28 @@
-// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-INVALID
-// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-SIMPLE
-// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COMPLEX
-// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-EMPTY
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
 
-// CHECK-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
-// CHECK-SIMPLE: fdebug-prefix-map=old=new
-// CHECK-COMPLEX: fdebug-prefix-map=old=n=ew
-// CHECK-EMPTY: fdebug-prefix-map=old=
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// 

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

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

As discussed with dankm on IRC, I still would like to see the correct behavior 
going into 8.0, i.e. not change it later. Since this also matters for potential 
faster implementations later, it seems like a good idea to do it now. The 
changes are well-localized.

(1) Do path prefix matching and not string prefix matching. The difference is 
that the file name must be longer than the prefix and the prefix must be 
followed by a path separator.
(2) The longest prefix match wins. Substituation is applied only once per file 
name, independent of the rules. This gives more predictable output and allows 
switching to a tree-lookup later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn accepted this revision.
Lekensteyn added a comment.

Still fine by me, thanks!

As for the commit message, perhaps reference:
https://bugs.llvm.org/show_bug.cgi?id=38135


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 3 inline comments as done.
dankm added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:612
+if (Map.find('=') == StringRef::npos)
+  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+else

Lekensteyn wrote:
> joerg wrote:
> > I'd prefer the bailout style here, i.e. `if (...) { D.diag(...); continue }`
> Wouldn't using `if (...) { D.diag(...); continue; }` also skip the 
> `A->claim()` call? Presumably that could result in spurious errors as well 
> about unused arguments?
It would, and did. I had @joerg's suggestion in an earlier patch on this review.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 181562.
dankm added a comment.

Restored original test case file names.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/debug-prefix-map.c
===
--- test/Driver/debug-prefix-map.c
+++ test/Driver/debug-prefix-map.c
@@ -1,9 +1,28 @@
-// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-INVALID
-// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-SIMPLE
-// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COMPLEX
-// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-EMPTY
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
 
-// CHECK-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
-// CHECK-SIMPLE: fdebug-prefix-map=old=new
-// CHECK-COMPLEX: fdebug-prefix-map=old=n=ew
-// CHECK-EMPTY: fdebug-prefix-map=old=
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-DEBUG-EMPTY: fdebug-prefix-map=old=
+// CHECK-MACRO-EMPTY: 

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn accepted this revision.
Lekensteyn added a comment.
This revision is now accepted and ready to land.

Tests pass here, using it on a large CMake project with a 
CMAKE_BUILD_TYPE=Debug and c/cxxflags `-ffile-prefix-map=$builddir= 
-ffile-prefix-map=$srcdir/= -fuse-ld=lld` successfully strips all traces of 
`$builddir` and `$srcdir`.

If you could take care of the previous comment (undo the rename or rename 
debug-prefix.map.c), then I've no further comments.

If @joerg or someone else could give the final review/pass, that would be great 
:)




Comment at: lib/Driver/ToolChains/Clang.cpp:612
+if (Map.find('=') == StringRef::npos)
+  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+else

joerg wrote:
> I'd prefer the bailout style here, i.e. `if (...) { D.diag(...); continue }`
Wouldn't using `if (...) { D.diag(...); continue; }` also skip the `A->claim()` 
call? Presumably that could result in spurious errors as well about unused 
arguments?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Except one thing, it looks reasonable to me. I'll try to run some tests and 
report back tomorrow.

(Not very familiar with Phabricator either. I still see some comments, 
hopefully the "Collapse" function does something useful here.)




Comment at: test/Driver/prefix-map.S:7
+
+// More tests for this flag in debug-prefix-map.c.

Maybe restore the old file name (debug-prefix-map.S) since this still tests the 
debug prefix functionality? And otherwise this comment needs to be updated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 4 inline comments as done.
dankm added a comment.

In D49466#1354468 , @Lekensteyn wrote:

> Could you add more tests to check the error message for bad options (missing 
> `=`):
>
>   -fdebug-prefix-map=bad
>   -fmacro-prefix-map=bad
>   -ffile-prefix-map=bad
>


Some more got added with the latest diff

> FWIW, GCC emits two errors for `-ffile-prefix-map=bad`.

Yes, this does too. It looked odd to me, but it's not a huge deal.

> Another edge case is `-ffile-prefix-map==foo/`, GCC currently uses this to 
> prepend `foo/` to every path. Not sure if that is intentional, but that is 
> the current behavior (one which is also replicated by this patch I believe).

Yes, with this patch it does that for file-prefix-map and macro-prefix-map. It 
already did that (sort-of) for debug-prefix-map, but seems to add it twice for 
some debugging information, but I'll fix that later since it's done that since 
at least version 5.0.

> Could you also mark review comments that are completed as "done"? It should 
> make the diff easier to read (I hope) :)

Yes, I tried to do that with this comment. I'm new to phabricator.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 181363.
dankm added a comment.

renamed err_drv_invalid_argument_to_prefix_map to 
err_drv_invalid_argument_to_option
added more frontend tests for macro-prefix-map and file-prefix-map.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/Driver/prefix-map.c
===
--- /dev/null
+++ test/Driver/prefix-map.c
@@ -0,0 +1,28 @@
+// RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
+// RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
+
+// RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+
+// RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+
+// RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+
+// CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
+// CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
+// CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
+// CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
+// CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-DEBUG-EMPTY: fdebug-prefix-map=old=
+// CHECK-MACRO-EMPTY: fmacro-prefix-map=old=
Index: test/Driver/prefix-map.S
===
--- /dev/null
+++ test/Driver/prefix-map.S
@@ -0,0 +1,7 @@
+// RUN: %clang -### -g -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s
+// RUN: %clang -### -g -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s
+
+// CHECK: cc1as
+// CHECK-SAME: -fdebug-prefix-map=old=new
+
+// More tests for this flag in debug-prefix-map.c.
Index: 

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

Could you add more tests to check the error message for bad options (missing 
`=`):

  -fdebug-prefix-map=bad
  -fmacro-prefix-map=bad
  -ffile-prefix-map=bad

FWIW, GCC emits two errors for `-ffile-prefix-map=bad`.

Another edge case is `-ffile-prefix-map==foo/`, GCC currently uses this to 
prepend `foo/` to every path. Not sure if that is intentional, but that is the 
current behavior (one which is also replicated by this patch I believe).

Could you also mark review comments that are completed as "done"? It should 
make the diff easier to read (I hope) :)




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:118
   "invalid deployment target for -stdlib=libc++ (requires %0 or later)">;
-def err_drv_invalid_argument_to_fdebug_prefix_map : Error<
-  "invalid argument '%0' to -fdebug-prefix-map">;
+def err_drv_invalid_argument_to_prefix_map : Error<
+  "invalid argument '%1' to -%0">;

Maybe rename `_to_prefix_map` to `_to_option`? (And maybe swap the order of 
parameters so `%0` comes before `%1`?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 181293.
dankm added a comment.

Made diagnostics for file-prefix-map display the actual option name.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,22 @@
+// 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
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/CodeGen/debug-prefix-map.c
===
--- test/CodeGen/debug-prefix-map.c
+++ test/CodeGen/debug-prefix-map.c
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH=empty %s -emit-llvm -o - | FileCheck %s -check-prefix CHECK-EVIL
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -main-file-name debug-prefix-map.c | FileCheck %s
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -fdebug-compilation-dir %p | FileCheck %s -check-prefix CHECK-COMPILATION-DIR
+// RUN: %clang -g -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang -g -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s
 
 #include "Inputs/stdio.h"
 
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -29,6 +29,7 @@
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorLexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
@@ -1456,6 +1457,16 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
+static void remapMacroPath(
+SmallString<128> ,
+const std::map>
+) {
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first)) {
+  Path = (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
+}
+}
+
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1523,6 +1534,7 @@
 if (PLoc.isValid()) {
   FN += PLoc.getFilename();
   Lexer::Stringify(FN);
+  remapMacroPath(FN, PPOpts->MacroPrefixMap);
   OS << '"' << FN << '"';
 }
 Tok.setKind(tok::string_literal);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3090,6 +3090,9 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ))
+Opts.MacroPrefixMap.insert(StringRef(A).split('='));
+
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
 StringRef Value(A->getValue());
 size_t Comma = Value.find(',');
Index: lib/Driver/ToolChains/Gnu.cpp

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

[Dan McGregor via Phabricator via cfe-commits]

dankm marked 2 inline comments as done.
dankm added a comment.

In D49466#1354238 , @Lekensteyn wrote:

> It would be nice to have this for Clang 8.0, the branch date is within 5 days 
> :)


Yup, that's why I'm ignoring a new baby for this :)




Comment at: lib/Driver/ToolChains/Clang.cpp:617
 if (Map.find('=') == StringRef::npos)
-  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+  D.Diag(diag::err_drv_invalid_argument_to_prefix_map) << Map << "debug";
 else

Lekensteyn wrote:
> For `clang -ffile-prefix-map=foo`, wouldn't this report `invalid argument 
> 'foo' to -fdebug-prefix-map`? If so, perhaps some method of `A` or 
> `A->getOption()` can be used?
Yes, it would seem so. It looks like A->getOption().getName() can be used.



Comment at: test/Preprocessor/file_test.c:5
+// RUN: %clang -E -fmacro-prefix-map=%p/= -c -o - %s | FileCheck %s 
--check-prefix CHECK-REMOVE
+//This is a comment
+

Lekensteyn wrote:
> Any reason to keep this comment?
Ha. No. That's from when I started writing this test. It can go away.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

It would be nice to have this for Clang 8.0, the branch date is within 5 days :)




Comment at: lib/Driver/ToolChains/Clang.cpp:617
 if (Map.find('=') == StringRef::npos)
-  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+  D.Diag(diag::err_drv_invalid_argument_to_prefix_map) << Map << "debug";
 else

For `clang -ffile-prefix-map=foo`, wouldn't this report `invalid argument 'foo' 
to -fdebug-prefix-map`? If so, perhaps some method of `A` or `A->getOption()` 
can be used?



Comment at: lib/Driver/ToolChains/Clang.cpp:630
+if (Map.find('=') == StringRef::npos)
+  D.Diag(diag::err_drv_invalid_argument_to_prefix_map) << Map << "macro";
+else

Same concern here about `-ffile-prefix-map=foo` showing an error message about 
`-fmacro-prefix-map`.



Comment at: test/Preprocessor/file_test.c:5
+// RUN: %clang -E -fmacro-prefix-map=%p/= -c -o - %s | FileCheck %s 
--check-prefix CHECK-REMOVE
+//This is a comment
+

Any reason to keep this comment?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

That's the other reason why I find the GCC specification as string prefix 
confusing. I still say we should just go with mapping of path names and then 
the order question mostly goes away.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm added a comment.

In D49466#1353437 , @alxu wrote:

> FYI, according to my comment on D49652 , 
> assuming I checked it correctly, gcc applies the maps in reverse order of 
> command line specification, not sorted order. It seems unlikely that anyone 
> is actually depending on the order though.


Yeah, I noticed that, but it appears to be undefined by GCC's documentation. I 
agree with review D49652 , but I also want to 
get this in before 8.0 branches, even if it's not ideal.

Right now we apply them in strict alphabetical order, switching to reverse 
order lets one map /objdir/ to / while remapping /objdir/ to 
/some/other/dir, which is my use case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu added a comment.

FYI, according to my comment on D49652 , 
assuming I checked it correctly, gcc applies the maps in reverse order of 
command line specification, not sorted order. It seems unlikely that anyone is 
actually depending on the order though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 181151.
dankm added a comment.

Added unit tests for the prefix remapping.

Switched the sorting on the prefix map, so that /sub gets remapped 
before  if both are specified.

I intend to do a more invasive change after this review to unify path prefix 
remapping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466

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

Index: test/Preprocessor/file_test.h
===
--- /dev/null
+++ test/Preprocessor/file_test.h
@@ -0,0 +1,2 @@
+filename: __FILE__
+basefile: __BASE_FILE__
Index: test/Preprocessor/file_test.c
===
--- /dev/null
+++ test/Preprocessor/file_test.c
@@ -0,0 +1,23 @@
+// 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
+//This is a comment
+
+filename: __FILE__
+#include "file_test.h"
+
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty{{[/\\]}}file_test.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{[/\\]}}file_test.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test.c"
+// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: basefile: "file_test.c"
+// CHECK-REMOVE-NOT: filename:
Index: test/CodeGen/debug-prefix-map.c
===
--- test/CodeGen/debug-prefix-map.c
+++ test/CodeGen/debug-prefix-map.c
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH=empty %s -emit-llvm -o - | FileCheck %s -check-prefix CHECK-EVIL
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -main-file-name debug-prefix-map.c | FileCheck %s
 // RUN: %clang_cc1 -debug-info-kind=standalone -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty %s -emit-llvm -o - -fdebug-compilation-dir %p | FileCheck %s -check-prefix CHECK-COMPILATION-DIR
+// RUN: %clang -g -fdebug-prefix-map=%p=/UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang -g -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s
 
 #include "Inputs/stdio.h"
 
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -29,6 +29,7 @@
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorLexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
@@ -1456,6 +1457,16 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
+static void remapMacroPath(
+SmallString<128> ,
+const std::map>
+) {
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first)) {
+  Path = (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
+}
+}
+
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1523,6 +1534,7 @@
 if (PLoc.isValid()) {
   FN += PLoc.getFilename();
   Lexer::Stringify(FN);
+  remapMacroPath(FN, PPOpts->MacroPrefixMap);
   OS << '"' << FN << '"';
 }
 Tok.setKind(tok::string_literal);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3090,6 +3090,9 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ))
+Opts.MacroPrefixMap.insert(StringRef(A).split('='));
+
   if (const Arg *A = 

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

[Dan McGregor via Phabricator via cfe-commits]

dankm added inline comments.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

Lekensteyn wrote:
> joerg wrote:
> > Lekensteyn wrote:
> > > dankm wrote:
> > > > dankm wrote:
> > > > > joerg wrote:
> > > > > > This doesn't handle directory vs string prefix prefix correctly, 
> > > > > > does it? At the very least, it should have a test case :)
> > > > > Good catch. I expect not. But on the other hand, it's exactly what 
> > > > > debug-prefix-map does :)
> > > > > 
> > > > > I'll add test cases in a future review. My first goal was getting 
> > > > > something sort-of working.
> > > > There should be a test, but apparently the debug prefix map code also 
> > > > does this.
> > > > 
> > > > What do you think the correct behaviour should be? a string prefix, or 
> > > > a directory prefix?
> > > It should be a string prefix (like GCC)
> > I disagree. I consider it a bug in GCC that it is a string prefix. It's 
> > quite inconsistent as well.
> I agree with you, it should have been a directory prefix but GCC implements 
> it as a string prefix although the GCC documents it as:
> "-fdebug-prefix-map=old=new When compiling files residing in **directory 
> old**, record debugging information describing them as if the files resided 
> in **directory new** instead."
> 
> If you decide to fix `-fmacro-prefix-map` to use a directory prefix match, 
> then the `-fdebug-prefix-map` should also be fixed for consistency. What 
> about implementing the (buggy) GCC-compatible behavior first and then fixing 
> both cases in a future patch? (I don't mind when the buggy behavior is fixed, 
> I just want to see this functionality moving forward.)
> 
> Another edge case that I ran into is when using the option to drop 
> directories. When using `-ffile-prefix-map=/src=`, the command `cd /src && cc 
> /src/foo.c` would have `__FILE__` equal to `/foo.c`. As a native "fix", one 
> would try `-ffile-prefix-map=/src/=` which indeed produces `__FILE__` equal 
> to `foo.c`.
> 
> Matching with a trailing slash however fails to correctly remap some debug 
> information, namely `DW_AT_comp_dir`. This contains the working directory 
> (`/src`) which is not matched by `/src/`. By using a proper directory prefix 
> match, this would be nicely fixed.
Yes, I'm going to submit my code with tests, and hoist the prefix remapping 
(for debug-prefix-map and macro-prefix-map) into a common location. Most 
probably part of Path.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm added a comment.

In D49466#1296790 , @ChristophBerg 
wrote:

> PostgreSQL 11 is now using LLVM to do JITing of SQL expressions. We'd need 
> this feature to strip the build directory off the .bc bitcode files so the 
> .deb packages build reproducibly.
>  @dankm: Are you still working on this? What can we do to help getting this 
> move forward?


I am. I'm about to push a new review. Sorry I missed this earlier.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49466



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

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

[Christoph Berg via Phabricator via cfe-commits]

ChristophBerg added a comment.

PostgreSQL 11 is now using LLVM to do JITing of SQL expressions. We'd need this 
feature to strip the build directory off the .bc bitcode files so the .deb 
packages build reproducibly.
@dankm: Are you still working on this? What can we do to help getting this move 
forward?


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added inline comments.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

joerg wrote:
> Lekensteyn wrote:
> > dankm wrote:
> > > dankm wrote:
> > > > joerg wrote:
> > > > > This doesn't handle directory vs string prefix prefix correctly, does 
> > > > > it? At the very least, it should have a test case :)
> > > > Good catch. I expect not. But on the other hand, it's exactly what 
> > > > debug-prefix-map does :)
> > > > 
> > > > I'll add test cases in a future review. My first goal was getting 
> > > > something sort-of working.
> > > There should be a test, but apparently the debug prefix map code also 
> > > does this.
> > > 
> > > What do you think the correct behaviour should be? a string prefix, or a 
> > > directory prefix?
> > It should be a string prefix (like GCC)
> I disagree. I consider it a bug in GCC that it is a string prefix. It's quite 
> inconsistent as well.
I agree with you, it should have been a directory prefix but GCC implements it 
as a string prefix although the GCC documents it as:
"-fdebug-prefix-map=old=new When compiling files residing in **directory old**, 
record debugging information describing them as if the files resided in 
**directory new** instead."

If you decide to fix `-fmacro-prefix-map` to use a directory prefix match, then 
the `-fdebug-prefix-map` should also be fixed for consistency. What about 
implementing the (buggy) GCC-compatible behavior first and then fixing both 
cases in a future patch? (I don't mind when the buggy behavior is fixed, I just 
want to see this functionality moving forward.)

Another edge case that I ran into is when using the option to drop directories. 
When using `-ffile-prefix-map=/src=`, the command `cd /src && cc /src/foo.c` 
would have `__FILE__` equal to `/foo.c`. As a native "fix", one would try 
`-ffile-prefix-map=/src/=` which indeed produces `__FILE__` equal to `foo.c`.

Matching with a trailing slash however fails to correctly remap some debug 
information, namely `DW_AT_comp_dir`. This contains the working directory 
(`/src`) which is not matched by `/src/`. By using a proper directory prefix 
match, this would be nicely fixed.


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

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

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

Lekensteyn wrote:
> dankm wrote:
> > dankm wrote:
> > > joerg wrote:
> > > > This doesn't handle directory vs string prefix prefix correctly, does 
> > > > it? At the very least, it should have a test case :)
> > > Good catch. I expect not. But on the other hand, it's exactly what 
> > > debug-prefix-map does :)
> > > 
> > > I'll add test cases in a future review. My first goal was getting 
> > > something sort-of working.
> > There should be a test, but apparently the debug prefix map code also does 
> > this.
> > 
> > What do you think the correct behaviour should be? a string prefix, or a 
> > directory prefix?
> It should be a string prefix (like GCC)
I disagree. I consider it a bug in GCC that it is a string prefix. It's quite 
inconsistent as well.


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

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

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn requested changes to this revision.
Lekensteyn added a comment.
This revision now requires changes to proceed.

The functionality looks correct to me, but could you include some tests in 
test/Driver/ and test/Preprocessor/ just to be sure?
test/Driver/debug-prefix-map.c and test/CodeGen/debug-prefix-map.c could serve 
as inspiration.

The documentation should probable be updated too: 
docs/ClangCommandLineReference.rst

(It would be nice to have this feature for Reproducible Builds)




Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

dankm wrote:
> dankm wrote:
> > joerg wrote:
> > > This doesn't handle directory vs string prefix prefix correctly, does it? 
> > > At the very least, it should have a test case :)
> > Good catch. I expect not. But on the other hand, it's exactly what 
> > debug-prefix-map does :)
> > 
> > I'll add test cases in a future review. My first goal was getting something 
> > sort-of working.
> There should be a test, but apparently the debug prefix map code also does 
> this.
> 
> What do you think the correct behaviour should be? a string prefix, or a 
> directory prefix?
It should be a string prefix (like GCC)


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

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

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

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

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu added a comment.

FYI, gcc uses the stupid and bad string prefix matching approach. if clang 
supports the same option, it should have the same behavior. you could decide 
that it's a bad idea, but then the option should be called something else, 
otherwise people will have to go back to testing compiler name and versions 
again which is :(.


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

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

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 156295.
dankm retitled this revision from "Initial implementation of 
-fmacro-prefix-mapand -ffile-prefix-map" to "Initial implementation of 
-fmacro-prefix-map and -ffile-prefix-map".
dankm edited the summary of this revision.

Repository:
  rC Clang

https://reviews.llvm.org/D49466

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp

Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -28,6 +28,7 @@
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorLexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/PTHLexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -1453,6 +1454,16 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
+static void remapMacroPath(SmallString<128> ,
+   const std::map ) {
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first)) {
+  Path = (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
+	  return;
+	}
+}
+
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1519,6 +1530,7 @@
 SmallString<128> FN;
 if (PLoc.isValid()) {
   FN += PLoc.getFilename();
+  remapMacroPath(FN, PPOpts->MacroPrefixMap);
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2847,6 +2847,9 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ))
+Opts.MacroPrefixMap.insert(StringRef(A).split('='));
+
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
 StringRef Value(A->getValue());
 size_t Comma = Value.find(',');
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -606,12 +606,28 @@
 
 /// Add a CC1 and CC1AS option to specify the debug file path prefix map.
 static void addDebugPrefixMapArg(const Driver , const ArgList , ArgStringList ) {
-  for (const Arg *A : Args.filtered(options::OPT_fdebug_prefix_map_EQ)) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
+options::OPT_fdebug_prefix_map_EQ)) {
 StringRef Map = A->getValue();
-if (Map.find('=') == StringRef::npos)
-  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
-else
-  CmdArgs.push_back(Args.MakeArgString("-fdebug-prefix-map=" + Map));
+if (Map.find('=') == StringRef::npos) {
+  D.Diag(diag::err_drv_invalid_argument_to_prefix_map) << Map << "debug";
+  continue;
+}
+CmdArgs.push_back(Args.MakeArgString("-fdebug-prefix-map=" + Map));
+A->claim();
+  }
+}
+
+/// Add a CC1 option to specify the macro file path prefix map.
+static void addMacroPrefixMapArg(const Driver , const ArgList , ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
+options::OPT_fmacro_prefix_map_EQ)) {
+StringRef Map = A->getValue();
+if (Map.find('=') == StringRef::npos) {
+  D.Diag(diag::err_drv_invalid_argument_to_prefix_map) << Map << "macro";
+  continue;
+}
+CmdArgs.push_back(Args.MakeArgString("-fmacro-prefix-map=" + Map));
 A->claim();
   }
 }
@@ -1223,6 +1239,8 @@
 // For IAMCU add special include arguments.
 getToolChain().AddIAMCUIncludeArgs(Args, CmdArgs);
   }
+
+  addMacroPrefixMapArg(D, Args, CmdArgs);
 }
 
 // FIXME: Move to target hook.
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -167,6 +167,9 @@
   /// build it again.
   std::shared_ptr FailedModules;
 
+  /// A prefix map for __FILE__ and __BASEFILE__
+  std::map MacroPrefixMap;
+
 public:
   PreprocessorOptions() : PrecompiledPreambleBytes(0, false) {}
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1755,10 +1755,16 @@
   Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the object/executable to