Page MenuHomePhabricator

[libc++] Fix conjunction/disjunction and mark a few LWG issues as complete
ClosedPublic

Authored by philnik on May 9 2022, 4:44 AM.

Details

Summary

Fixes #54803
Fixes #53133

Diff Detail

Event Timeline

philnik created this revision.May 9 2022, 4:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2022, 4:44 AM
Herald added a subscriber: mgorny. · View Herald Transcript
philnik requested review of this revision.May 9 2022, 4:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2022, 4:44 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 428040.May 9 2022, 5:14 AM
  • Generate files
philnik updated this revision to Diff 428060.May 9 2022, 6:20 AM
  • Fix order
ldionne requested changes to this revision.May 11 2022, 9:31 AM
ldionne added inline comments.
libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp
2

I think we are missing tests for conjunction and disjunction with types that don't have a ::value but should never be instantiated because of short-circuiting.

71

We should be testing that is_base_of instead, since that's what the spec requests. Same for disjunction.

This revision now requires changes to proceed.May 11 2022, 9:31 AM
philnik updated this revision to Diff 428878.May 12 2022, 1:37 AM
philnik marked 2 inline comments as done.
  • Address comments
ldionne requested changes to this revision.May 18 2022, 1:22 PM
ldionne added inline comments.
libcxx/include/__type_traits/conjunction.h
27

It would be more idiomatic to use using type = ...; here and below.

libcxx/include/type_traits
524–530

I would consider moving these helpers to conjunction.h and disjunction.h in the future, with a small comment explaining how they differ from disjunction and conjunction.

libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp
8

Can you please make sure that we have coverage for all the cases that were reported in https://github.com/llvm/llvm-project/issues/54803? The bug report has some nice tests we can easily add to our test suite in one form or another.

10

From live review: let's also make sure we allow types that have only an explicit cast to bool, since the standard says bool(Bi::value) (https://eel.is/c++draft/meta.logical).

77

Please add something like

static_assert(!std::conjunction<False, HasNoValue, True>::value);

We need to make sure that we can instantiate it, not only use is_base_of on it (yes, I know is_base_of will instantiate it, but that's only how our implementation works). Same applies for disjunction.

This revision now requires changes to proceed.May 18 2022, 1:22 PM
philnik updated this revision to Diff 430914.May 20 2022, 2:02 AM
philnik marked 5 inline comments as done.
  • Address comments
philnik updated this revision to Diff 431181.May 21 2022, 3:10 PM
  • Try to fix GCC
ldionne accepted this revision.Thu, Jun 2, 8:59 AM
This revision is now accepted and ready to land.Thu, Jun 2, 8:59 AM