[libcxx] [test] Fix windows errors in fs.op.rename

[• Quuxplusone]

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.rename/rename.pass.cpp

66

(1) Please add a trailing comma on line 67.
(2) I'd feel better if the comment were phrased in terms of the standard-specified behavior: is libc++'s current Win32 behavior conforming, or does libc++ actually need to go put a check inside std::filesystem::rename to deal with the moving-a-directory-onto-a-file situation because Win32 doesn't appropriately reject it? Can you find the chapter and verse that describes this behavior in the standard?

[mstorsjo]

Expanded the comment with spec references, added a requested trailing comma.

[mstorsjo]

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.rename/rename.pass.cpp

66

1. Done

2. Expanded the comment. As far as I can see from http://eel.is/c++draft/fs.op.rename, exactly which cases are treated as errors is left to be implementation/platform dependent, and the note explicitly calls out that renaming (an unspecified type of path object) over a non-directory is allowed to succeed and remove the non-directory (i.e. regular file).

[• Quuxplusone]

LGTM! Although, if you wanted to scope-creep it, it might be nice to add a positive test for Windows' actual behavior. Right now IIUC you're making Windows lose all test coverage for this (interesting) case.

[mstorsjo]

In D98640#2628968, @Quuxplusone wrote:

LGTM! Although, if you wanted to scope-creep it, it might be nice to add a positive test for Windows' actual behavior. Right now IIUC you're making Windows lose all test coverage for this (interesting) case.

I can look into doing that later as follow-up. I see that the test otherwise also is lacking positive tests for renaming directories, and negative tests for dir->dir renames.

[mstorsjo]

Ping for a second approval

[curdeius]

LGTM.

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.rename/rename.pass.cpp

#endif
     const path file = env.create_file("file1", 42);
     const path dir = env.create_dir("dir1");
     struct TestCase {
       path from;
       path to;
     } cases[] = {
         {dne, dne},
         {file, dir},
-        {dir, file}
+#ifndef _WIN32
+        // The spec doesn't say that this case must be an error; fs.op.rename
+        // note 1.2.1 says that a file may be overwritten by a rename.
+        // On Windows, with rename() implemented with MoveFileExW, overwriting
+        // a file with a directory is not an error.
+        {dir, file},
+#endif
     };
     for (auto& TC : cases) {
         auto from_before = status(TC.from);
         auto to_before = status(TC.to);
         std::error_code ec;
         rename(TC.from, TC.to, ec);
         TEST_REQUIRE(ec);
         TEST_CHECK(from_before.type() == status(TC.from).type());