-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Remove non-Standard <experimental/filesystem>
#5765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
If someone's looking at Standard filesystem.cpp, we don't need to keep a comment pointing out Experimental filesys.cpp.
I don't think it's necessary to transfer this to the cryopreserved experimental_filesystem.hpp.
… `filesystem::path` detection. After removing support for `experimental::filesystem::path`, we can simplify away `_Is_any_path`. Now, those constructors can be guarded for `_HAS_CXX17`, and they can use `is_same_v` exactly as depicted by N5014 [ifstream.cons]/4, [ofstream.cons]/4, [fstream.cons]/4. They remain templates, so there's no ABI impact to their definitions completely vanishing in C++14 mode.
Add a TRANSITION, ABI comment. We don't need to define the silencing macro, because we can't include `<experimental/filesystem>` anymore. We use `wstring`, so we need to include `<string>`. (This was previously dragged in, at least partially.) We use `uint64_t` and `uintmax_t`, so we should include `<cstdint>` to be good kitties. We need the `_MAX_FILESYS_NAME`, `_FS_BEGIN`, and `_FS_END` macros. Within the namespace, we need `file_type`, `perms` and its `_BITMASK_OPS`, and `space_info`.
tests/tr1/tests/filesystem1/test.cpp is still allowed to say that it's testing `<experimental/filesystem>`; rephrasing this wasn't worth the effort.
…hpp and dual-mode test Standard filesystem. It was previously dragging in `<experimental/filesystem>` via `<__msvc_all_public_headers.hpp>`, which is why I missed it in microsoft#5749.
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔪 🔪 🔪 <experimental/filesystem> 🔪 🔪 🔪
LGTM
|
I've pushed a merge with |
Overview
This permanently removes our non-Standard
<experimental/filesystem>header, which was superseded by C++17<filesystem>.For 6 years (VS 2019 16.3 in September 2019),
<experimental/filesystem>has been "hard deprecated" with an impossible-to-ignore compiler error telling users that "The<experimental/filesystem>header providingstd::experimental::filesystemis deprecated by Microsoft and will be REMOVED." It's finally time to deliver on that promise. (Also, the name inherently indicated that the technology was tentative and not eternal.)Eliminating this code removes unnecessary complexity from this fiendishly complex library. For users, it removes ways to write non-portable code. For library implementers, it slightly reduces our maintenance burden, making it easier for us to reason about the remaining Standard code.
The interface of
<experimental/filesystem>is fairly close to that of Standard<filesystem>, making migration easy in most cases. However, the implementation of<experimental/filesystem>was riddled with deficiencies and limitations (includingMAX_PATHlimitations). This is a major reason to push users to migrate to Standard<filesystem>, whose implementation was written from scratch to be conforming. Unfortunately, we can't completely remove the<experimental/filesystem>implementation, as its separately compiled component must be retained for binary compatibility. But we can still remove the header itself, and some support logic in other headers.Because of the retained-for-bincompat caveat, I am preserving test coverage for
<experimental/filesystem>. Most of our retained-for-bincompat functions have no test coverage, and we simply rely on not changing them, but the implementation of<experimental/filesystem>was large and complicated enough that I think preserving test coverage is worth the effort.Commits
tests/std/include/experimental_filesystem.hpp<experimental/filesystem>from product code.filesystem.cpp, we don't need to keep a comment pointing out Experimentalfilesys.cpp.__cpp_lib_experimental_filesystem.experimental_filesystem.hpp._FSTREAM_SUPPORTS_EXPERIMENTAL_FILESYSTEM, simplify Standardfilesystem::pathdetection.experimental::filesystem::path, we can simplify away_Is_any_path. Now, those constructors can be guarded for_HAS_CXX17, and they can useis_same_vexactly as depicted by WG21-N5014 [ifstream.cons]/4, [ofstream.cons]/4, [fstream.cons]/4. They remain templates, so there's no ABI impact to their definitions completely vanishing in C++14 mode.src/filesys.cppto compile standalone.<experimental/filesystem>anymore.wstring, so we need to include<string>. (This was previously dragged in, at least partially.)uint64_tanduintmax_t, so we should include<cstdint>to be good kitties._MAX_FILESYS_NAME,_FS_BEGIN, and_FS_ENDmacros.file_type,permsand its_BITMASK_OPS, andspace_info.include_each_header_alone_matrix.lst.experimental_filesystem.hpp.tests/tr1/tests/filesystem1/test.cppis still allowed to say that it's testing<experimental/filesystem>;rephrasing this wasn't worth the effort.
Dev09_172666_tr1_tuple_odrto includeexperimental_filesystem.hppand dual-mode test Standard filesystem.<experimental/filesystem>via<__msvc_all_public_headers.hpp>, which is why I missed it in Improve<filesystem>test coverage #5749.