This is an archive of the discontinued LLVM Phabricator instance.

Differential D112650

[libc++] Make __decay_copy constexpr
ClosedPublic

Authored by ldionne on Oct 27 2021, 12:13 PM.

Details

Reviewers
Mordante
• Quuxplusone
Group Reviewers
Restricted Project
Commits
rG2999b7307f70: [libc++] Make __decay_copy constexpr
Summary

This is going to be necessary to implement some range adaptors.
As a fly-by fix, rename _LIBCPP_INLINE_VISIBILITY to _LIBCPP_HIDE_FROM_ABI
and remove a redundant inline keyword.

Diff Detail

Event Timeline

ldionne requested review of this revision.Oct 27 2021, 12:13 PM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2021, 12:13 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
jloser added a subscriber: jloser.Oct 27 2021, 12:35 PM
jloser added inline comments.
libcxx/include/__utility/decay_copy.h
25

I'm assuming this is used in C++11 code (thread and future) which is why we spell typename decay<_Tp>::type instead of decay_t<_Tp>?

27

Nit: pre-existing, but can we remove the extra space to cuddle the >>, please?

Mordante accepted this revision as: Mordante.Oct 27 2021, 12:51 PM
Mordante added a subscriber: Mordante.

LGTM!

libcxx/include/__utility/decay_copy.h
27

Just FYI this is due to clang-tidy still being used in C++03 mode. We can change the clang-tidy language standard after switching to clang-tidy-13. (I expect make a patch for it real soon.)

jloser added inline comments.Oct 27 2021, 12:54 PM
libcxx/include/__utility/decay_copy.h
27

Thanks for the context. I'd definitely be in favor of that change. :)

jloser added inline comments.Oct 27 2021, 12:57 PM
libcxx/include/__utility/decay_copy.h
27

Is clang-tidy even run on libc++ CI? It's mentioned in libcxx/TODO.TXT to run clang-tidy on libc++. I notice that most llvm projects have their own .clang-tidy file, but libc++ does not.

• Quuxplusone accepted this revision.Oct 27 2021, 1:20 PM
• Quuxplusone added a subscriber: • Quuxplusone.
• Quuxplusone added inline comments.
libcxx/include/__utility/decay_copy.h
27

Typo for clang-format, I assume. :) +1 for cuddling >>.

This revision is now accepted and ready to land.Oct 27 2021, 1:20 PM
Mordante added inline comments.Oct 27 2021, 1:23 PM
libcxx/include/__utility/decay_copy.h
27

Note to self, don't read about clang-tidy while writing about clang-format ;-)

Harbormaster completed remote builds in B131010: Diff 382749.Oct 27 2021, 1:49 PM
ldionne marked 5 inline comments as done.Oct 27 2021, 2:31 PM

Thanks for the reviews everyone, I'll ship with the applied comments.

libcxx/include/__utility/decay_copy.h
25

Yes!

Closed by commit rG2999b7307f70: [libc++] Make __decay_copy constexpr (authored by ldionne). · Explain WhyOct 27 2021, 2:32 PM
This revision was automatically updated to reflect the committed changes.
ldionne marked an inline comment as done.
ldionne added a commit: rG2999b7307f70: [libc++] Make __decay_copy constexpr.

Revision Contents

PathSize
libcxx/
include/
__utility/
5 lines

Diff 382800

libcxx/include/__utility/decay_copy.h

Show All 15 Lines
#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 _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp> template <class _Tp>
inline _LIBCPP_INLINE_VISIBILITY typename decay<_Tp>::type __decay_copy(_Tp&& __t) _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
typename decay<_Tp>::type __decay_copy(_Tp&& __t)
jloserUnsubmitted
Done
ReplyInline Actions

I'm assuming this is used in C++11 code (thread and future) which is why we spell typename decay<_Tp>::type instead of decay_t<_Tp>?

jloser: I'm assuming this is used in C++11 code (thread and future) which is why we spell `typename…
ldionneAuthorUnsubmitted
Done
ReplyInline Actions

Yes!

ldionne: Yes!
#if _LIBCPP_STD_VER > 17 #if _LIBCPP_STD_VER > 17
noexcept(is_nothrow_convertible_v<_Tp, remove_reference_t<_Tp> >) noexcept(is_nothrow_convertible_v<_Tp, remove_reference_t<_Tp>>)
jloserUnsubmitted
Done
ReplyInline Actions

Nit: pre-existing, but can we remove the extra space to cuddle the >>, please?

jloser: Nit: pre-existing, but can we remove the extra space to cuddle the `>>`, please?
MordanteUnsubmitted
Done
ReplyInline Actions

Just FYI this is due to clang-tidy still being used in C++03 mode. We can change the clang-tidy language standard after switching to clang-tidy-13. (I expect make a patch for it real soon.)

Mordante: Just FYI this is due to clang-tidy still being used in C++03 mode. We can change the clang-tidy…
jloserUnsubmitted
Done
ReplyInline Actions

Thanks for the context. I'd definitely be in favor of that change. :)

jloser: Thanks for the context. I'd definitely be in favor of that change. :)
jloserUnsubmitted
Done
ReplyInline Actions

Is clang-tidy even run on libc++ CI? It's mentioned in libcxx/TODO.TXT to run clang-tidy on libc++. I notice that most llvm projects have their own .clang-tidy file, but libc++ does not.

jloser: Is `clang-tidy` even run on `libc++` CI? It's mentioned in `libcxx/TODO.TXT` to run `clang…
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

Typo for clang-format, I assume. :) +1 for cuddling >>.

Quuxplusone: Typo for `clang-format`, I assume. :) +1 for cuddling `>>`.
MordanteUnsubmitted
Done
ReplyInline Actions

Note to self, don't read about clang-tidy while writing about clang-format ;-)

Mordante: Note to self, don't read about clang-tidy while writing about clang-format ;-)
#endif #endif
{ {
return _VSTD::forward<_Tp>(__t); return _VSTD::forward<_Tp>(__t);
} }
_LIBCPP_END_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___UTILITY_DECAY_COPY_H #endif // _LIBCPP___UTILITY_DECAY_COPY_H