This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Run `substitutes-in-compile-flags.sh.cpp` test on Windows.
ClosedPublic

Authored by vvereschaka on May 6 2021, 9:46 PM.

Details

Summary

Fix for substitutes-in-compile-flags.sh.cpp to run it properly on Windows platform.

Diff Detail

Event Timeline

vvereschaka requested review of this revision.May 6 2021, 9:46 PM
vvereschaka created this revision.
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 6 2021, 9:46 PM

Hi @ldionne,

these changes should work on Windows, Linux and Darwin platforms, but I'm not sure about the others. In case of any problems we can disable this test for Windows with https://reviews.llvm.org/D102045.

Currently this test still gets failed on the windows to linux cross builders, as example: https://lab.llvm.org/buildbot/#/builders/119/builds/3724

curdeius added inline comments.
libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp
9–11

This comment needs to be adapted/removed.

ldionne accepted this revision.May 7 2021, 7:42 AM

Generally LGTM with a few comments. Ship it once reviewer feedback has been applied.

libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp
17–18

Can you name the file something like %t.escaped.grep instead? It makes it more obvious that the only purpose is to escape paths.

19

This isn't needed. We generally don't clean up the temporary files in the tests.

This revision is now accepted and ready to land.May 7 2021, 7:42 AM

@ldionne, @curdeius thank you for review.
Updated diff accordingly

  • reworded comment
  • %t.grep => %t.escaped.grep
  • do not remove temporary file in the test.