[llvm-branch-commits] [libcxx] e4ed349 - [libc++] [P1164] [C++20] Make fs::create_directory() error if there is already a non-directory.

Marek Kurdej via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Dec 9 23:44:30 PST 2020


Author: Marek Kurdej
Date: 2020-12-10T08:40:27+01:00
New Revision: e4ed349c765827a824cb38ec6ef3447263b768cf

URL: https://github.com/llvm/llvm-project/commit/e4ed349c765827a824cb38ec6ef3447263b768cf
DIFF: https://github.com/llvm/llvm-project/commit/e4ed349c765827a824cb38ec6ef3447263b768cf.diff

LOG: [libc++] [P1164] [C++20] Make fs::create_directory() error if there is already a non-directory.

Also mark LWG2935 and LWG3079 as complete.

Applied retroactively to previous standards too, as it's a DR.

* https://wg21.link/P1164
* https://wg21.link/lwg2935
* https://wg21.link/lwg3079

Reviewed By: ldionne, #libc

Differential Revision: https://reviews.llvm.org/D92769

Added: 
    

Modified: 
    libcxx/docs/Cxx2aStatusIssuesStatus.csv
    libcxx/docs/Cxx2aStatusPaperStatus.csv
    libcxx/src/filesystem/operations.cpp
    libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp
    libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/Cxx2aStatusIssuesStatus.csv b/libcxx/docs/Cxx2aStatusIssuesStatus.csv
index aa5d93a55f6b..55a9d9b4818c 100644
--- a/libcxx/docs/Cxx2aStatusIssuesStatus.csv
+++ b/libcxx/docs/Cxx2aStatusIssuesStatus.csv
@@ -15,7 +15,7 @@
 "","","","",""
 "`2779 <https://wg21.link/LWG2779>`__","[networking.ts] Relax requirements on buffer sequence iterators","Albuquerque","",""
 "`2870 <https://wg21.link/LWG2870>`__","Default value of parameter theta of polar should be dependent","Albuquerque","|Complete|",""
-"`2935 <https://wg21.link/LWG2935>`__","What should create_directories do when p already exists but is not a directory?","Albuquerque","",""
+"`2935 <https://wg21.link/LWG2935>`__","What should create_directories do when p already exists but is not a directory?","Albuquerque","|Nothing To Do|",""
 "`2941 <https://wg21.link/LWG2941>`__","[thread.req.timing] wording should apply to both member and namespace-level functions","Albuquerque","|Nothing To Do|",""
 "`2944 <https://wg21.link/LWG2944>`__","LWG 2905 accidentally removed requirement that construction of the deleter doesn't throw an exception","Albuquerque","|Nothing To Do|",""
 "`2945 <https://wg21.link/LWG2945>`__","Order of template parameters in optional comparisons","Albuquerque","|Complete|",""
@@ -83,7 +83,7 @@
 "`3071 <https://wg21.link/LWG3071>`__","[networking.ts] read_until still refers to ""input sequence""","Rapperswil","|Nothing To Do|",""
 "`3074 <https://wg21.link/LWG3074>`__","Non-member functions for valarray should only deduce from the valarray","Rapperswil","",""
 "`3076 <https://wg21.link/LWG3076>`__","basic_string CTAD ambiguity","Rapperswil","|Complete|",""
-"`3079 <https://wg21.link/LWG3079>`__","LWG 2935 forgot to fix the existing_p overloads of create_directory","Rapperswil","",""
+"`3079 <https://wg21.link/LWG3079>`__","LWG 2935 forgot to fix the existing_p overloads of create_directory","Rapperswil","|Nothing To Do|",""
 "`3080 <https://wg21.link/LWG3080>`__","Floating point from_chars pattern specification breaks round-tripping","Rapperswil","",""
 "`3083 <https://wg21.link/LWG3083>`__","What should ios::iword(-1) do?","Rapperswil","|Nothing To Do|",""
 "`3094 <https://wg21.link/LWG3094>`__","[time.duration.io]p4 makes surprising claims about encoding","Rapperswil","",""

diff  --git a/libcxx/docs/Cxx2aStatusPaperStatus.csv b/libcxx/docs/Cxx2aStatusPaperStatus.csv
index 4efad23981cf..c495e0210cf7 100644
--- a/libcxx/docs/Cxx2aStatusPaperStatus.csv
+++ b/libcxx/docs/Cxx2aStatusPaperStatus.csv
@@ -87,7 +87,7 @@
 "`P0920R2 <https://wg21.link/P0920R2>`__","LWG","Precalculated hash values in lookup","Kona","Reverted by `P1661 <https://wg21.link/P1661>`__",""
 "`P1001R2 <https://wg21.link/P1001R2>`__","LWG","Target Vectorization Policies from Parallelism V2 TS to C++20","Kona","",""
 "`P1024R3 <https://wg21.link/P1024R3>`__","LWG","Usability Enhancements for std::span","Kona","|Complete|","9.0"
-"`P1164R1 <https://wg21.link/P1164R1>`__","LWG","Make create_directory() Intuitive","Kona","",""
+"`P1164R1 <https://wg21.link/P1164R1>`__","LWG","Make create_directory() Intuitive","Kona","|Complete|","12.0"
 "`P1227R2 <https://wg21.link/P1227R2>`__","LWG","Signed ssize() functions, unsigned size() functions","Kona","|Complete|","9.0"
 "`P1252R2 <https://wg21.link/P1252R2>`__","LWG","Ranges Design Cleanup","Kona","",""
 "`P1286R2 <https://wg21.link/P1286R2>`__","CWG","Contra CWG DR1778","Kona","",""

diff  --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index 6a259792477c..fb27d54cf653 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -849,8 +849,17 @@ bool __create_directory(const path& p, error_code* ec) {
 
   if (::mkdir(p.c_str(), static_cast<int>(perms::all)) == 0)
     return true;
-  if (errno != EEXIST)
+
+  if (errno == EEXIST) {
+    error_code mec = capture_errno();
+    error_code ignored_ec;
+    const file_status st = status(p, ignored_ec);
+    if (!is_directory(st)) {
+      err.report(mec);
+    }
+  } else {
     err.report(capture_errno());
+  }
   return false;
 }
 
@@ -868,8 +877,17 @@ bool __create_directory(path const& p, path const& attributes, error_code* ec) {
 
   if (::mkdir(p.c_str(), attr_stat.st_mode) == 0)
     return true;
-  if (errno != EEXIST)
+
+  if (errno == EEXIST) {
+    error_code mec = capture_errno();
+    error_code ignored_ec;
+    const file_status st = status(p, ignored_ec);
+    if (!is_directory(st)) {
+      err.report(mec);
+    }
+  } else {
     err.report(capture_errno());
+  }
   return false;
 }
 

diff  --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp
index 17ad65001daa..b5b1a8cca198 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp
@@ -8,6 +8,9 @@
 
 // UNSUPPORTED: c++03
 
+// This test requires the dylib support introduced in D92769.
+// XFAIL: with_system_cxx_lib=macosx10.15
+
 // <filesystem>
 
 // bool create_directory(const path& p);
@@ -95,8 +98,40 @@ TEST_CASE(dest_is_file)
     const path file = env.create_file("file", 42);
     std::error_code ec = GetTestEC();
     TEST_CHECK(fs::create_directory(file, ec) == false);
-    TEST_CHECK(!ec);
+    TEST_CHECK(ec);
+    TEST_CHECK(is_regular_file(file));
+}
+
+TEST_CASE(dest_part_is_file)
+{
+    scoped_test_env env;
+    const path file = env.create_file("file");
+    const path dir = env.make_env_path("file/dir1");
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(fs::create_directory(dir, ec) == false);
+    TEST_CHECK(ec);
     TEST_CHECK(is_regular_file(file));
+    TEST_CHECK(!exists(dir));
+}
+
+TEST_CASE(dest_is_symlink_to_dir)
+{
+    scoped_test_env env;
+    const path dir = env.create_dir("dir");
+    const path sym = env.create_symlink(dir, "sym_name");
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(create_directory(sym, ec) == false);
+    TEST_CHECK(!ec);
+}
+
+TEST_CASE(dest_is_symlink_to_file)
+{
+    scoped_test_env env;
+    const path file = env.create_file("file");
+    const path sym = env.create_symlink(file, "sym_name");
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(create_directory(sym, ec) == false);
+    TEST_CHECK(ec);
 }
 
 TEST_SUITE_END()

diff  --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp
index a58fa8e2c4b9..83f9c8d27444 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp
@@ -8,6 +8,9 @@
 
 // UNSUPPORTED: c++03
 
+// This test requires the dylib support introduced in D92769.
+// XFAIL: with_system_cxx_lib=macosx10.15
+
 // <filesystem>
 
 // bool create_directory(const path& p, const path& attr);
@@ -95,10 +98,23 @@ TEST_CASE(dest_is_file)
     const path attr_dir = env.create_dir("attr_dir");
     std::error_code ec = GetTestEC();
     TEST_CHECK(fs::create_directory(file, attr_dir, ec) == false);
-    TEST_CHECK(!ec);
+    TEST_CHECK(ec);
     TEST_CHECK(is_regular_file(file));
 }
 
+TEST_CASE(dest_part_is_file)
+{
+    scoped_test_env env;
+    const path file = env.create_file("file", 42);
+    const path dir = env.make_env_path("file/dir1");
+    const path attr_dir = env.create_dir("attr_dir");
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(fs::create_directory(dir, attr_dir, ec) == false);
+    TEST_CHECK(ec);
+    TEST_CHECK(is_regular_file(file));
+    TEST_CHECK(!exists(dir));
+}
+
 TEST_CASE(attr_dir_is_invalid) {
   scoped_test_env env;
   const path file = env.create_file("file", 42);
@@ -117,15 +133,39 @@ TEST_CASE(attr_dir_is_invalid) {
   }
 }
 
-TEST_CASE(dest_is_symlink) {
+TEST_CASE(dest_is_symlink_to_unexisting) {
   scoped_test_env env;
-  const path dir = env.create_dir("dir");
+  const path attr_dir = env.create_dir("attr_dir");
   const path sym = env.create_symlink("dne_sym", "dne_sym_name");
   {
     std::error_code ec = GetTestEC();
-    TEST_CHECK(create_directory(sym, dir, ec) == false);
+    TEST_CHECK(create_directory(sym, attr_dir, ec) == false);
+    TEST_CHECK(ec);
+  }
+}
+
+TEST_CASE(dest_is_symlink_to_dir) {
+  scoped_test_env env;
+  const path dir = env.create_dir("dir");
+  const path sym = env.create_symlink(dir, "sym_name");
+  const path attr_dir = env.create_dir("attr_dir");
+  {
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(create_directory(sym, attr_dir, ec) == false);
     TEST_CHECK(!ec);
   }
 }
 
+TEST_CASE(dest_is_symlink_to_file) {
+  scoped_test_env env;
+  const path file = env.create_file("file");
+  const path sym = env.create_symlink(file, "sym_name");
+  const path attr_dir = env.create_dir("attr_dir");
+  {
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(create_directory(sym, attr_dir, ec) == false);
+    TEST_CHECK(ec);
+  }
+}
+
 TEST_SUITE_END()


        


More information about the llvm-branch-commits mailing list