This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Fixes concepts overload resolution.
ClosedPublic

Authored by Mordante on Apr 20 2022, 9:53 AM.

Details

Reviewers
royjacobson
Group Reviewers
Restricted Project
Commits
rG7a98d8351b27: [libc++] Fixes concepts overload resolution.
Summary

D123182 fixes a bug in Clang's overload resolution. After it landed it
was discovered basic_format_arg's constructors contains this bug. This
fixes the bug in libc++, unblocking D123182.

The code has been tested in combination with D123182.

Diff Detail

Event Timeline

Mordante created this revision.Apr 20 2022, 9:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2022, 9:53 AM
Mordante requested review of this revision.Apr 20 2022, 9:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2022, 9:53 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Thank you for the quick fix!

LGTM, small nit about the bool& change.

libcxx/include/__format/format_arg.h
163

I think here it's unnecessary because it's a specific type, but maybe you just preferred consistency?

royjacobson accepted this revision.Apr 20 2022, 10:04 AM
Mordante marked an inline comment as done.Apr 20 2022, 10:22 AM

Thanks for the review!

libcxx/include/__format/format_arg.h
163

This matches the wording of http://eel.is/c++draft/format.arg closer. (Note I use a const reference instead of an rvalue reference. This was the wording of an earlier revision of the Standard, changing it to an rvalue reference is work in progress.)

Mordante accepted this revision as: Restricted Project.Apr 23 2022, 9:18 AM
Mordante marked an inline comment as done.

The fix is trivial so unblock the patch waiting for this.

This revision is now accepted and ready to land.Apr 23 2022, 9:18 AM
This revision was landed with ongoing or failed builds.Apr 23 2022, 9:19 AM
This revision was automatically updated to reflect the committed changes.