This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Granularize __mutex_base
ClosedPublic

Authored by philnik on Mar 16 2023, 8:16 AM.

Details

Reviewers
ldionne
Mordante
EricWF
Group Reviewers
Restricted Project
Commits
rGe655d8a54880: [libc++] Granularize __mutex_base
Summary

This also updates the moved code to the current style. (i.e. _VSTD -> std, _LIBCPP_INLINE_VISIBILITY -> _LIBCPP_HIDE_FROM_ABI, clang-format).

Diff Detail

Event Timeline

philnik created this revision.Mar 16 2023, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2023, 8:16 AM
philnik requested review of this revision.Mar 16 2023, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2023, 8:16 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 506368.Mar 19 2023, 2:43 AM

Format granularized headers

philnik edited the summary of this revision. (Show Details)Mar 19 2023, 2:45 AM
Mordante requested changes to this revision.Mar 19 2023, 11:38 AM

I really dislike mixing granularizing patches with other changes. For the future please only do one change per patch.
I'm fine with only CI testing the followup changes and self-approving them. I don't feel it's needed to undo the changes for now.

I want to see it again to see the changes to the transitive includes.

libcxx/test/libcxx/transitive_includes/cxx2b.csv
120

Merge conflicts

This revision now requires changes to proceed.Mar 19 2023, 11:38 AM
philnik updated this revision to Diff 506432.Mar 19 2023, 3:06 PM
philnik marked an inline comment as done.

Try to fix CI

philnik updated this revision to Diff 506446.Mar 19 2023, 3:57 PM

Try to fix CI

EricWF accepted this revision.Mar 20 2023, 11:58 AM

I agree with @Mordante

These sorts of changes really muddy up the history, and are hard to review.

This revision is now accepted and ready to land.Mar 21 2023, 12:31 PM
This revision was landed with ongoing or failed builds.Mar 22 2023, 12:17 PM
This revision was automatically updated to reflect the committed changes.
miyuki added a subscriber: miyuki.Mar 23 2023, 8:36 AM

Hi. This change causes failures in one of our configurations, compiled with _LIBCPP_HAS_NO_THREADS. Compilation of mutex.cpp fails with the following error:

[2023-03-22T20:37:03.871Z] In file included from /workspace/workspace/armcompiler/next/toolchain/src/llvm-project/libcxx/src/mutex.cpp:11:
[2023-03-22T20:37:03.871Z] In file included from /workspace/workspace/armcompiler/next/toolchain/build/libcxx_libcxxabi_libunwind/armv8.1m.main_hard_nofp_mve_armlibc/src/libcxx_libcxxabi_libunwind_armv8.1m.main_hard_nofp_mve_armlibc-build/include/c++/v1/mutex:192:
[2023-03-22T20:37:03.871Z] /workspace/workspace/armcompiler/next/toolchain/build/libcxx_libcxxabi_libunwind/armv8.1m.main_hard_nofp_mve_armlibc/src/libcxx_libcxxabi_libunwind_armv8.1m.main_hard_nofp_mve_armlibc-build/include/c++/v1/__condition_variable/condition_variable.h:94:44: error: no template named 'is_floating_point'; did you mean 'std::is_floating_point'?
[2023-03-22T20:37:03.871Z] inline _LIBCPP_HIDE_FROM_ABI __enable_if_t<is_floating_point<_Rep>::value, chrono::nanoseconds>
[2023-03-22T20:37:03.871Z]                                            ^~~~~~~~~~~~~~~~~

[2023-03-22T20:37:03.871Z]                                            std::is_floating_point

This happens because in the file __condition_variable/condition_variable.h, the _LIBCPP_BEGIN_NAMESPACE_STD line is guarded with #ifndef _LIBCPP_HAS_NO_THREADS so when the file is used with _LIBCPP_HAS_NO_THREADS the function __safe_nanosecond_cast gets defined in the global namespace (instead of std) from which is_floating_point is not accessible via unqualified lookup.

Could you please fix this issue?

Hi. This change causes failures in one of our configurations, compiled with _LIBCPP_HAS_NO_THREADS. Compilation of mutex.cpp fails with the following error:

[2023-03-22T20:37:03.871Z] In file included from /workspace/workspace/armcompiler/next/toolchain/src/llvm-project/libcxx/src/mutex.cpp:11:
[2023-03-22T20:37:03.871Z] In file included from /workspace/workspace/armcompiler/next/toolchain/build/libcxx_libcxxabi_libunwind/armv8.1m.main_hard_nofp_mve_armlibc/src/libcxx_libcxxabi_libunwind_armv8.1m.main_hard_nofp_mve_armlibc-build/include/c++/v1/mutex:192:
[2023-03-22T20:37:03.871Z] /workspace/workspace/armcompiler/next/toolchain/build/libcxx_libcxxabi_libunwind/armv8.1m.main_hard_nofp_mve_armlibc/src/libcxx_libcxxabi_libunwind_armv8.1m.main_hard_nofp_mve_armlibc-build/include/c++/v1/__condition_variable/condition_variable.h:94:44: error: no template named 'is_floating_point'; did you mean 'std::is_floating_point'?
[2023-03-22T20:37:03.871Z] inline _LIBCPP_HIDE_FROM_ABI __enable_if_t<is_floating_point<_Rep>::value, chrono::nanoseconds>
[2023-03-22T20:37:03.871Z]                                            ^~~~~~~~~~~~~~~~~

[2023-03-22T20:37:03.871Z]                                            std::is_floating_point

This happens because in the file __condition_variable/condition_variable.h, the _LIBCPP_BEGIN_NAMESPACE_STD line is guarded with #ifndef _LIBCPP_HAS_NO_THREADS so when the file is used with _LIBCPP_HAS_NO_THREADS the function __safe_nanosecond_cast gets defined in the global namespace (instead of std) from which is_floating_point is not accessible via unqualified lookup.

Could you please fix this issue?

This is fixed by D146682. I hope @glandium lands it soon.