[PATCH] D76869: [Clang] Restore replace_path_prefix instead of startswith

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 29 09:38:53 PDT 2020


MaskRay added inline comments.


================
Comment at: clang/test/Preprocessor/file_test_windows.c:12
+// CHECK: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK: filename: "A:\\UNLIKELY_PATH\\empty{{/|\\\\}}Inputs/include-file-test/file_test.h"
+// CHECK: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
----------------
saudi wrote:
> MaskRay wrote:
> > Can `{{/|\\\\}}` be simplified?
> Yes, however it would be `/`, which looks odd as compared to the line above that uses `\` for the `file_test_windows.c` file.
> This is because of how the include file full paths are resolved, where `/` are introduced. 
> 
> Should I try to fix that behavior? 
> Or maybe I could leave this test with `/` instead of `{{/|\\\\}}`, leaving the coherence fix for a separate patch. What do you think?
The coherence fix can be made in a separate patch. When that get fixes, the updated test will clearly show which has been change.


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

https://reviews.llvm.org/D76869





More information about the llvm-commits mailing list