This is an archive of the discontinued LLVM Phabricator instance.

Change `tuple_size` from a `class` to a `struct` (see PR#39871)
ClosedPublic

Authored by mclow.lists on Dec 7 2018, 5:11 PM.

Details

Reviewers
EricWF
ldionne
Summary

In the standard, it's a mix of things.
libc++ currently consistently implements this as a class.
I talked to the editors, and Jonathan believes that this can be fixed editorially.
I'm assuming that it will end up as a struct, and so here's a patch to implement that.

This will affect people who have specialized std::tuple_size for their own uses, and compile with -Wmismatched-tags.
It will also affect people who use libc++ with MS compilers, where the ABI is different between struct and class

I don't recommend landing this until the change goes into the standard - which will probably be a week or so.
But I was in the neighborhood, and I figured I'd write this up.

Diff Detail

Event Timeline

mclow.lists created this revision.Dec 7 2018, 5:11 PM
EricWF accepted this revision.Dec 10 2018, 8:46 AM

LGTM.

Consider adding a *.fail.cpp test that declares a specialization as a class and checks for

// expected-error {{tuple_size' defined as a class template here but previously declared as a struct template}}
This revision is now accepted and ready to land.Dec 10 2018, 8:46 AM
ldionne accepted this revision.Dec 10 2018, 10:45 AM

Oh gosh I've been wondering forever why we defined them as classes in the standard. Glad to see we're fixing that.

I'm waiting for the (editorial) change to go into the standard; once that happens, I will commit this.

EricWF added a comment.Jan 5 2019, 5:36 PM

Should we commit this before the next release? Maybe ping Wakly to get the editorial fix landed?

Should we commit this before the next release? Maybe ping Wakly to get the editorial fix landed?

I will ping Jonathan about this today.

Oh, look, this is https://github.com/cplusplus/draft/issues/534, where JW wrote:

Compilers do not compile the standard, they compile headers that come with their implementation. The implementation headers can use whatever class-keys they want to, they don't have to match the standard, and changing the standard doesn't mean implementations will change.

I say commit this.

mclow.lists closed this revision.Jan 11 2019, 2:02 PM

Committed as revision 350972.