This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Overhaul std::quoted; fix its relationship to character traits.
ClosedPublic

Authored by Quuxplusone on Feb 18 2022, 8:31 AM.

Details

Summary

https://en.cppreference.com/w/cpp/io/manip/quoted

Move __quoted_output_proxy into the one file that uses it. (This was the top of the rabbit hole... :))

A const char* has no associated traits class, so std::quoted("literal") should be printable into any basic_ostream regardless of traits.

Use hidden-friend operator<< and operator>>, since we're permitted to. (The exact signature is unspecified because the class itself is unspecified.)

We shouldn't support std::quoted("literal") in C++03 or C++11 mode. (We do need std::__quoted(s) and std::__quoted(cs) in C++11 mode, because they're used by std::__fs::filesystem::path.)

Use _LIBCPP_HIDDEN and _LIBCPP_HIDE_FROM_ABI consistently.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Feb 18 2022, 8:31 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptFeb 18 2022, 8:31 AM

If you have multiple commits for this mess it would be nice to have multiple PRs.

libcxx/include/__filesystem/path.h
971
libcxx/include/iomanip
521
525
579
Quuxplusone marked 3 inline comments as done.Feb 21 2022, 11:28 AM

If you have multiple commits for this mess it would be nice to have multiple PRs.

Sadly I don't. Most of the changes end up being interrelated anyway (e.g. operator<< can be a hidden friend, but also we change its signature at the same time; __quoted_output_proxy moves, but also goes under an #if at the same time...). I could probably disentangle the original "move __quoted_output_proxy to <iomanip>" change into its own commit, but that might be about all.

libcxx/include/__filesystem/path.h
971

Here I'm naturally inclined to match line 963, but I could go either way.

libcxx/include/iomanip
525

Yup, looks like (; is libc++ style. Will switch.

579

Here I'll stick with Standardese style (which is also, generally speaking, libc++ style): int *p, int& r.

596–597

I've added _LIBCPP_HIDE_FROM_ABI to this ctor. That's correct, right? @ldionne

ldionne requested changes to this revision.Mar 4 2022, 11:07 AM

Is there any way you can land the style changes (whitespace and other consistency stuff) as a separate NFC commit, and re-upload this one so we can take a look?

libcxx/include/iomanip
596–597

Yes, that looks correct to me.

libcxx/test/std/input.output/iostream.format/quoted.manip/quoted_traits.compile.pass.cpp
9

We already have libcxx/test/std/input.output/iostream.format/quoted.manip/quoted.pass.cpp. Can you please add a comment explaining what this is actually testing, since it's obviously different from libcxx/test/std/input.output/iostream.format/quoted.manip/quoted.pass.cpp.

145

I think that's what you mean -- same above.

This revision now requires changes to proceed.Mar 4 2022, 11:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2022, 11:07 AM
Quuxplusone marked 2 inline comments as done.
Quuxplusone edited the summary of this revision. (Show Details)

Push a lot of the non-functional diffs (and the ADL-proofing) to main in small separate commits, and then rebase.

Undo a few diffs I'm not married to.

ldionne accepted this revision.Mar 7 2022, 9:18 AM
This revision is now accepted and ready to land.Mar 7 2022, 9:18 AM

Can you add a release note for the removal of quoted("literal") before C++14?

This revision was landed with ongoing or failed builds.Mar 7 2022, 10:29 AM
This revision was automatically updated to reflect the committed changes.