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

[Louis Dionne via Phabricator via libcxx-commits]

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

> I'm not sure if it should be applied retroactively to C++17 too (as I've done currently).

Yes, we apply LWG resolutions retroactively to all standards.

Also, you'll need markup for some Apple system libraries that shipped Filesystem but don't contain that fix. They are not covered by CI right now but they will once I land https://reviews.llvm.org/D92794. I'll let you know and you can rebase on top of it -- it should be trivial to add the `UNSUPPORTED` annotations.



================
Comment at: libcxx/docs/Cxx2aStatusIssuesStatus.csv:18
 "`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","|Complete|","12.0"
 "`2941 <https://wg21.link/LWG2941>`__","[thread.req.timing] wording should apply to both member and namespace-level functions","Albuquerque","|Nothing To Do|",""
----------------
Isn't that "Nothing to do"?


================
Comment at: libcxx/docs/Cxx2aStatusIssuesStatus.csv:86
 "`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","|Complete|","12.0"
 "`3080 <https://wg21.link/LWG3080>`__","Floating point from_chars pattern specification breaks round-tripping","Rapperswil","",""
----------------
Same here -- isn't that "Nothing to do"?


================
Comment at: libcxx/src/filesystem/operations.cpp:858
+    if (!is_directory(st)) {
+      err.report(mec); // Or err.report(errc::not_a_directory); ?
+    }
----------------
curdeius wrote:
> Question to reviewers, what should we return, `error_code` corresponding to `errno` (`mec`) or another error code, e.g `not_a_directory`?
I believe an error code corresponding to `errno` is the most sensible thing, since that's effectively surfacing the actual failure reported by `mkdir` to the user, which is what we want IMO. Also, the Standard gives us leeway here, it only seems to say that we need to report an error -- so either way appears to be conforming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92769