Page MenuHomePhabricator

[libc++] Enable rvalue overloads for pair in C++03

Authored by philnik on Aug 31 2022, 4:58 AM.



We require rvalue support anyways, so let's use it.

Diff Detail

Event Timeline

philnik created this revision.Aug 31 2022, 4:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 4:58 AM
philnik requested review of this revision.Aug 31 2022, 4:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 4:58 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
huixie90 added inline comments.Aug 31 2022, 10:04 AM

I guess this somehow changed the c++ 03 behaviour. it used to make copies on 1. function parameters 2. constructor initialisers
Now it only make the copy inside constructors. Is it ok to make this change?

ldionne accepted this revision.Sep 2 2022, 8:47 AM
ldionne added inline comments.

I was going to say this:

I don't think it is, actually, because in C++03 make_pair is explicitly specified like

template< class T1, class T2 >
std::pair<T1,T2> make_pair( T1 t, T2 u );

IMO the rest of these changes are OK, but this one does changes our C++03 API (which one day we will deprecate if my dreams become reality).

But then, @philnik reminded me that libc++ was originally built to implement the C++11 standard even on top of a C++03 compiler, and that is actually one of the things we tell users. In other words, we never really faithfully implement C++03 anyways. For example, we provide all kinds of things like std::shared_ptr in C++03 mode, and those are all C++11 features.

So yeah, I think it's fine to drop this. And it only reinforces my desire to get a discussion started on deprecating C++03 support entirely.


This one is definitely good. std::get(pair) was added in C++11/14 depending on the overloads, and we probably only did that before rvalue references as a C++03 extension in Clang.


Side comment, but we do seem to have quite a few files that haven't moved to the new license. You could make a NFC with those without review if you want.

This revision is now accepted and ready to land.Sep 2 2022, 8:47 AM
philnik updated this revision to Diff 457683.Sep 2 2022, 1:03 PM
philnik marked 2 inline comments as done.
  • Try to fix CI
philnik updated this revision to Diff 457925.Sep 5 2022, 3:44 AM
  • Next try
This revision was automatically updated to reflect the committed changes.