This is an archive of the discontinued LLVM Phabricator instance.

Differential D151490

[libc++] Gives ignore external linkage.
ClosedPublic

Authored by Mordante on May 25 2023, 1:01 PM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG5d4281d5493c: [libc++] Gives ignore external linkage.
Summary

A slightly different fix is in D144994.

Diff Detail

Unit TestsFailed

TimeTest
19,830 mslibcxx CI - C++26 > llvm-libc++-shared-cfg-in.libcxx::transitive_includes.sh.cpp

Event Timeline

Mordante created this revision.May 25 2023, 1:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2023, 1:01 PM
Harbormaster completed remote builds in B234617: Diff 525762.May 25 2023, 5:19 PM
Mordante updated this revision to Diff 526066.May 26 2023, 8:03 AM

Rebased to fix CI.

Harbormaster completed remote builds in B234863: Diff 526066.May 26 2023, 11:44 AM
Mordante published this revision for review.May 26 2023, 11:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2023, 11:50 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
ldionne accepted this revision.May 30 2023, 8:48 AM
This revision is now accepted and ready to land.May 30 2023, 8:48 AM
philnik added a subscriber: philnik.May 30 2023, 8:57 AM
philnik added inline comments.
libcxx/include/tuple
1514–1517

Is the anonymous namespace required? It should already have internal linkage, right?

Mordante marked an inline comment as done.May 30 2023, 9:48 AM
Mordante added inline comments.
libcxx/include/tuple
1514–1517

I think it's not, but I want to keep the diff minimal.

Closed by commit rG5d4281d5493c: [libc++] Gives ignore external linkage. (authored by Mordante). · Explain WhyMay 31 2023, 8:23 AM
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Mordante added a commit: rG5d4281d5493c: [libc++] Gives ignore external linkage..

Revision Contents

PathSize
libcxx/
include/
4 lines

Diff 525762

libcxx/include/tuple

Show First 20 Lines • Show All 1,502 Lines • ▼ Show 20 Lines
template <class _Up> template <class _Up>
struct __ignore_t struct __ignore_t
{ {
template <class _Tp> template <class _Tp>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
const __ignore_t& operator=(_Tp&&) const {return *this;} const __ignore_t& operator=(_Tp&&) const {return *this;}
}; };
# if _LIBCPP_STD_VER >= 17
inline constexpr __ignore_t<unsigned char> ignore = __ignore_t<unsigned char>();
# else
namespace { namespace {
constexpr __ignore_t<unsigned char> ignore = __ignore_t<unsigned char>(); constexpr __ignore_t<unsigned char> ignore = __ignore_t<unsigned char>();
} // namespace } // namespace
# endif
philnikUnsubmitted
Done
ReplyInline Actions

Is the anonymous namespace required? It should already have internal linkage, right?

philnik: Is the anonymous namespace required? It should already have internal linkage, right?
MordanteAuthorUnsubmitted
Done
ReplyInline Actions

I think it's not, but I want to keep the diff minimal.

Mordante: I think it's not, but I want to keep the diff minimal.
template <class... _Tp> template <class... _Tp>
inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
tuple<typename __unwrap_ref_decay<_Tp>::type...> tuple<typename __unwrap_ref_decay<_Tp>::type...>
make_tuple(_Tp&&... __t) make_tuple(_Tp&&... __t)
{ {
return tuple<typename __unwrap_ref_decay<_Tp>::type...>(_VSTD::forward<_Tp>(__t)...); return tuple<typename __unwrap_ref_decay<_Tp>::type...>(_VSTD::forward<_Tp>(__t)...);
} }
▲ Show 20 Lines • Show All 343 Lines • Show Last 20 Lines