This is an archive of the discontinued LLVM Phabricator instance.

[libc++][format] Improves CTAD.
ClosedPublic

Authored by Mordante on Oct 5 2022, 10:56 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG261b5abf72cf: [libc++][format] Improves CTAD.
Summary

This partly reverts D133535 and enables CTAD for more parts in format.

Diff Detail

Event Timeline

Mordante created this revision.Oct 5 2022, 10:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 5 2022, 10:56 AM
Mordante published this revision for review.Oct 8 2022, 5:51 AM
Mordante added a reviewer: ldionne.
Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2022, 5:51 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Oct 11 2022, 9:27 AM
ldionne added inline comments.
libcxx/include/__config
1197–1198
1204–1206

In a follow-up patch, I think we could consider also renaming this macro to _LIBCPP_CTAD_INTENDED_TO_WORK? We can bikeshed on that but I think we can find something better than _LIBCPP_CTAD_SUPPORTED_FOR_TYPE.

1204–1207

Can we instead try this?

#define _LIBCPP_CTAD_SUPPORTED_FOR_TYPE(_ClassName) \
  template <class ..._Tag> \
  _ClassName(typename _Tag::__allow_ctad...) -> _ClassName<_Tag...>;

Would that allow handling class templates that take any number of arguments?

This revision is now accepted and ready to land.Oct 11 2022, 9:27 AM
Mordante updated this revision to Diff 466857.Oct 11 2022, 10:37 AM
Mordante marked an inline comment as done.

Rebased and addresses review comments.

Mordante marked an inline comment as done.Oct 11 2022, 10:38 AM
Mordante added inline comments.
libcxx/include/__config
1204–1207

I tried this locally and it works, let's see what the CI thinks.

This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.