This is an archive of the discontinued LLVM Phabricator instance.

[libc++][NFC] Remove workaround for variadic templates in C++03
ClosedPublic

Authored by ldionne on Aug 9 2021, 10:28 AM.

Details

Summary

That's not necessary anymore, since we always build the dylib with
C++11 capabilities nowadays.

Diff Detail

Event Timeline

ldionne requested review of this revision.Aug 9 2021, 10:28 AM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2021, 10:28 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Quuxplusone added a subscriber: Quuxplusone.

LGTM % whitespace comment, FWIW.

libcxx/src/locale.cpp
84–86

As in that other recent review, please prefer libc++ style: class... Args and Args... args.

ldionne accepted this revision.Aug 9 2021, 3:05 PM
ldionne marked an inline comment as done.

The windows job failed due to "no space left on device", so I'll assume this passed.

libcxx/src/locale.cpp
84–86

There's no "libc++" style in this case. I get 652 hits for class ..., and 940 hits for class..., so there's no clearly consistent approach here. If you can explain to me why class... is better, I'll change it.

This revision is now accepted and ready to land.Aug 9 2021, 3:05 PM
This revision was landed with ongoing or failed builds.Aug 9 2021, 3:06 PM
This revision was automatically updated to reflect the committed changes.
ldionne marked an inline comment as done.
Quuxplusone added inline comments.Aug 9 2021, 4:42 PM
libcxx/src/locale.cpp
84–86

If you can explain to me why class... is better, I'll change it.

Only the stylistic argument that it's "what we've always done" (and what you've done as recently as D101277, although I guess that started out as Chris's patch). I do see now that we've got a mix in libc++; class... predominates but not overwhelmingly. Likewise Ts&&... ts mildly predominates over Ts&& ...ts (and overwhelmingly over Ts &&...ts, of course; we also have 8 instances of Ts &&... ts).

The Standard draft itself does use class... Ts (and T& t, and west const) as its house style:

$ git grep 'class[.][.][.] [A-Z]' | wc -l
     424
$ git grep 'class [.][.][.][A-Z]' | wc -l
       1
$ git grep 'typename[.][.][.] [A-Z]' | wc -l
      10
$ git grep 'typename [.][.][.][A-Z]' | wc -l
       5

Arguably, <class... Ts> has the minor benefit that when you textually delete the trailing parameter name, you get <class...>, which we do overwhelmingly prefer over <class ...>:

$ git grep '[^ ][.][.][.]>' libcxx/include/ | wc -l
     778
$ git grep '[ ][.][.][.]>' libcxx/include/ | wc -l
       9

Anyway, I'll try to stop commenting on this. I really just noticed you doing it on the patch the other day; I'd never noticed it before this week.