This is an archive of the discontinued LLVM Phabricator instance.

Differential D109075

[libc++][NFC] Move concepts to a subheader.
ClosedPublic

Authored by Mordante on Sep 1 2021, 10:47 AM.

Details

Reviewers
• Quuxplusone
ldionne
Group Reviewers
Restricted Project
Commits
rG163792113400: [libc++][NFC] Move concepts to a subheader.
Summary

D103357 added some new concepts. Since the header <concepts> has moved
all its concepts to a separate header these new concepts feel out of
place. Move them to the appropriate header.

Diff Detail

Event Timeline

Mordante requested review of this revision.Sep 1 2021, 10:47 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2021, 10:47 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B122120: Diff 369987.Sep 1 2021, 10:47 AM
Mordante updated this revision to Diff 370005.Sep 1 2021, 11:45 AM

Retrigger CI.

Harbormaster completed remote builds in B122134: Diff 370005.Sep 1 2021, 11:46 AM
• Quuxplusone requested changes to this revision.Sep 1 2021, 11:52 AM
• Quuxplusone added a subscriber: • Quuxplusone.
• Quuxplusone added inline comments.
libcxx/include/__concepts/arithmetic.h
43–44

__libcpp_floating_point is completely unused, as is __libcpp_is_floating_point. I believe you should just remove those.
Anyone who wants to know whether a type is float/double/long-double should just use the standard std::is_floating_point trait or std::floating_point concept. We have no reason to provide a separate version of our own.

libcxx/include/type_traits
791–792

This comment seems no longer useful. Just remove lines 792-794.

This revision now requires changes to proceed.Sep 1 2021, 11:52 AM
Mordante marked 2 inline comments as done.Sep 1 2021, 11:16 PM
Mordante added inline comments.
libcxx/include/__concepts/arithmetic.h
43–44

I added it for consistency, but I don't feel strongly about this concept. I don't like to do that change in this patch. I'll do this in a separate commit.

libcxx/include/type_traits
791–792

Good catch, must have happened when rebasing the original patch.

Mordante updated this revision to Diff 370279.Sep 2 2021, 8:19 AM
Mordante marked 2 inline comments as done.

Remove outdated comment.

• Quuxplusone accepted this revision as: • Quuxplusone.Sep 2 2021, 8:59 AM

LGTM now!

Harbormaster completed remote builds in B122314: Diff 370279.Sep 2 2021, 9:26 AM
ldionne accepted this revision.Sep 2 2021, 9:55 AM
This revision is now accepted and ready to land.Sep 2 2021, 9:55 AM
Closed by commit rG163792113400: [libc++][NFC] Move concepts to a subheader. (authored by Mordante). · Explain WhySep 2 2021, 10:22 AM
This revision was automatically updated to reflect the committed changes.
Mordante added a commit: rG163792113400: [libc++][NFC] Move concepts to a subheader..
Mordante mentioned this in rG803141c7a794: [libc++] Remove an unused internal concept..Sep 2 2021, 10:28 AM

Revision Contents

PathSize
libcxx/
include/
__concepts/
9 lines
__format/
2 lines
18 lines
3 lines

Diff 370318

libcxx/include/__concepts/arithmetic.h

Show All 28 Lines
concept signed_integral = integral<_Tp> && is_signed_v<_Tp>; concept signed_integral = integral<_Tp> && is_signed_v<_Tp>;
template<class _Tp> template<class _Tp>
concept unsigned_integral = integral<_Tp> && !signed_integral<_Tp>; concept unsigned_integral = integral<_Tp> && !signed_integral<_Tp>;
template<class _Tp> template<class _Tp>
concept floating_point = is_floating_point_v<_Tp>; concept floating_point = is_floating_point_v<_Tp>;
// Concept helpers for the internal type traits for the fundamental types.
template <class _Tp>
concept __libcpp_unsigned_integer = __libcpp_is_unsigned_integer<_Tp>::value;
template <class _Tp>
concept __libcpp_signed_integer = __libcpp_is_signed_integer<_Tp>::value;
template <class _Tp>
concept __libcpp_floating_point = __libcpp_is_floating_point<_Tp>::value;
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

__libcpp_floating_point is completely unused, as is __libcpp_is_floating_point. I believe you should just remove those.
Anyone who wants to know whether a type is float/double/long-double should just use the standard std::is_floating_point trait or std::floating_point concept. We have no reason to provide a separate version of our own.

Quuxplusone: `__libcpp_floating_point` is completely unused, as is `__libcpp_is_floating_point`. I believe…
MordanteAuthorUnsubmitted
Done
ReplyInline Actions

I added it for consistency, but I don't feel strongly about this concept. I don't like to do that change in this patch. I'll do this in a separate commit.

Mordante: I added it for consistency, but I don't feel strongly about this concept. I don't like to do…
#endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CONCEPTS) #endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CONCEPTS)
_LIBCPP_END_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___CONCEPTS_ARITHMETIC_H #endif // _LIBCPP___CONCEPTS_ARITHMETIC_H

libcxx/include/__format/format_arg.h

// -*- C++ -*- // -*- C++ -*-
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#ifndef _LIBCPP___FORMAT_FORMAT_ARG_H #ifndef _LIBCPP___FORMAT_FORMAT_ARG_H
#define _LIBCPP___FORMAT_FORMAT_ARG_H #define _LIBCPP___FORMAT_FORMAT_ARG_H
#include <__concepts/arithmetic.h>
#include <__config> #include <__config>
#include <__format/format_error.h> #include <__format/format_error.h>
#include <__format/format_fwd.h> #include <__format/format_fwd.h>
#include <__functional_base> #include <__functional_base>
#include <__variant/monostate.h> #include <__variant/monostate.h>
#include <concepts>
#include <string> #include <string>
#include <string_view> #include <string_view>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#pragma GCC system_header #pragma GCC system_header
#endif #endif
_LIBCPP_PUSH_MACROS _LIBCPP_PUSH_MACROS
▲ Show 20 Lines • Show All 230 Lines • Show Last 20 Lines

libcxx/include/concepts

Show First 20 Lines • Show All 146 Lines • ▼ Show 20 Lines
#include <__concepts/predicate.h> #include <__concepts/predicate.h>
#include <__concepts/regular.h> #include <__concepts/regular.h>
#include <__concepts/relation.h> #include <__concepts/relation.h>
#include <__concepts/same_as.h> #include <__concepts/same_as.h>
#include <__concepts/semiregular.h> #include <__concepts/semiregular.h>
#include <__concepts/swappable.h> #include <__concepts/swappable.h>
#include <__concepts/totally_ordered.h> #include <__concepts/totally_ordered.h>
#include <__config> #include <__config>
#include <type_traits>
#include <version> #include <version>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#pragma GCC system_header #pragma GCC system_header
#endif #endif
_LIBCPP_BEGIN_NAMESPACE_STD
#if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CONCEPTS)
// Concept helpers for the internal type traits for the fundamental types.
template <class _Tp>
concept __libcpp_unsigned_integer = __libcpp_is_unsigned_integer<_Tp>::value;
template <class _Tp>
concept __libcpp_signed_integer = __libcpp_is_signed_integer<_Tp>::value;
template <class _Tp>
concept __libcpp_floating_point = __libcpp_is_floating_point<_Tp>::value;
#endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CONCEPTS)
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP_CONCEPTS #endif // _LIBCPP_CONCEPTS

libcxx/include/type_traits

Show First 20 Lines • Show All 782 Lines • ▼ Show 20 Lines
#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES) #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
template <class _Tp> template <class _Tp>
_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_integral_v _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_integral_v
= is_integral<_Tp>::value; = is_integral<_Tp>::value;
#endif #endif
#endif // __has_keyword(__is_integral) #endif // __has_keyword(__is_integral)
// __libcpp_is_signed_integer, __libcpp_is_unsigned_integer
// <concepts> implements __libcpp_signed_integer, __libcpp_unsigned_integer
// [basic.fundamental] defines five standard signed integer types; // [basic.fundamental] defines five standard signed integer types;
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

This comment seems no longer useful. Just remove lines 792-794.

Quuxplusone: This comment seems no longer useful. Just remove lines 792-794.
MordanteAuthorUnsubmitted
Done
ReplyInline Actions

Good catch, must have happened when rebasing the original patch.

Mordante: Good catch, must have happened when rebasing the original patch.
// __int128_t is an extended signed integer type. // __int128_t is an extended signed integer type.
// The signed and unsigned integer types, plus bool and the // The signed and unsigned integer types, plus bool and the
// five types with "char" in their name, compose the "integral" types. // five types with "char" in their name, compose the "integral" types.
template <class _Tp> struct __libcpp_is_signed_integer : public false_type {}; template <class _Tp> struct __libcpp_is_signed_integer : public false_type {};
template <> struct __libcpp_is_signed_integer<signed char> : public true_type {}; template <> struct __libcpp_is_signed_integer<signed char> : public true_type {};
template <> struct __libcpp_is_signed_integer<signed short> : public true_type {}; template <> struct __libcpp_is_signed_integer<signed short> : public true_type {};
template <> struct __libcpp_is_signed_integer<signed int> : public true_type {}; template <> struct __libcpp_is_signed_integer<signed int> : public true_type {};
▲ Show 20 Lines • Show All 3,476 Lines • Show Last 20 Lines