The operations.cpp file contained the implementation of a ton of
functionality unrelated to just the filesystem operations. Splitting
this up into more files will make it possible in the future to support
parts of <filesystem> (e.g. path) on systems where there is no notion
of a filesystem.
Details
- Reviewers
philnik Mordante - Group Reviewers
Restricted Project - Commits
- rGc7d3c84449f4: [libc++] Split sources for <filesystem>
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM with green CI. Do you plant to update this to the current style in a follow-up patch?
I didn't plan to, but I could. It looks like there aren't that many changes required to make it comply to our clang-format. I'll look into it later but I'd like to avoid doing this before landing my stack of changes cause I want to avoid rebase nightmares.
LGTM, when the CI is happy. On small comment.
libcxx/src/filesystem/directory_entry.cpp | ||
---|---|---|
10 | Please also check the other new files. |
We're seeing some failures in our CI that seem related to this patch. I think either some CMAKE needs adjusting or maybe there is a stale path in the preprocessor directives that cause something to be missed.
Error Message:
FAILED: libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/operations.cpp.obj /b/s/w/ir/x/w/staging/llvm_build/./bin/clang++ --target=x86_64-unknown-fuchsia --sysroot=/b/s/w/ir/x/w/sdk/arch/x64/sysroot -DLIBCXX_BUILDING_LIBCXXABI -D_GLIBCXX_ASSERTIONS -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS -D_LIBCPP_ENABLE_ASSERTIONS -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/s/w/ir/x/w/llvm-llvm-project/libcxx/src -I/b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/include/c++/v1 -I/b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/include/x86_64-unknown-fuchsia/c++/v1 -I/b/s/w/ir/x/w/llvm-llvm-project/libcxxabi/include -resource-dir=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/./lib/clang/17 --target=x86_64-unknown-fuchsia -I/b/s/w/ir/x/w/sdk/pkg/sync/include -I/b/s/w/ir/x/w/sdk/pkg/fdio/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections -fdata-sections -ffile-prefix-map=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/runtimes/runtimes-x86_64-unknown-fuchsia-bins=../staging/llvm_build/tools/clang/stage2-bins/runtimes/runtimes-x86_64-unknown-fuchsia-bins -ffile-prefix-map=/b/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes -O2 -g -DNDEBUG -std=c++20 -fPIC -UNDEBUG -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -MD -MT libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/operations.cpp.obj -MF libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/operations.cpp.obj.d -o libcxx/src/CMakeFiles/cxx_shared.dir/filesystem/operations.cpp.obj -c /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/operations.cpp In file included from /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/operations.cpp:25: /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/time_utils.h:263:16: error: incomplete result type 'TimeVal' (aka 'std::filesystem::detail::(anonymous namespace)::timeval') in function definition 263 | inline TimeVal make_timeval(TimeSpec const& ts) { | ^ /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/posix_compat.h:119:24: note: forward declaration of 'std::filesystem::detail::(anonymous namespace)::timeval' 119 | using TimeVal = struct timeval; | ^ In file included from /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/operations.cpp:25: /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/time_utils.h:266:54: error: member access into incomplete type 'std::filesystem::detail::(anonymous namespace)::timeval' 266 | using int_type = decltype(std::declval<TimeVal>().tv_usec); | ^ /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/posix_compat.h:119:24: note: forward declaration of 'std::filesystem::detail::(anonymous namespace)::timeval' 119 | using TimeVal = struct timeval; | ^ In file included from /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/operations.cpp:25: /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/time_utils.h:268:24: error: unknown type name 'int_type' 268 | return static_cast<int_type>(dur); | ^ /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/time_utils.h:270:11: error: variable has incomplete type 'TimeVal' (aka 'std::filesystem::detail::(anonymous namespace)::timeval') 270 | TimeVal TV = {}; | ^ /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/posix_compat.h:119:24: note: forward declaration of 'std::filesystem::detail::(anonymous namespace)::timeval' 119 | using TimeVal = struct timeval; | ^ In file included from /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/operations.cpp:25: /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/time_utils.h:278:11: error: variable has incomplete type 'TimeVal' (aka 'std::filesystem::detail::(anonymous namespace)::timeval') 278 | TimeVal ConvertedTS[2] = {make_timeval(TS[0]), make_timeval(TS[1])}; | ^ /b/s/w/ir/x/w/llvm-llvm-project/libcxx/src/filesystem/posix_compat.h:119:24: note: forward declaration of 'std::filesystem::detail::(anonymous namespace)::timeval' 119 | using TimeVal = struct timeval; | ^ 5 errors generated.
Chatted w/ @phosek offline and he thinks this may require some changes to the Fuchsia headers as the failure is happening in Fuchsia targets. Sorry for the noise on this, as I thought this was happening in the normal Linux part of the toolchain build. If we find that there will be some changes required on the libcxx side we'll let you know, but you can ignore my previous comments for now.
it also caused:
https://github.com/llvm/llvm-project/issues/63613
/usr/include/sys/statvfs.h:47:1: error: unknown type name '__BEGIN_DECLS' 47 | __BEGIN_DECLS
Please also check the other new files.