This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Implement DR2386 for C++17 structured binding
ClosedPublic

Authored by rnk on Aug 9 2019, 4:54 PM.

Details

Summary

Allow implementations to provide complete definitions of
std::tuple_size<T>, but to omit the 'value' member to signal that T is
not tuple-like. The Microsoft standard library implements
std::tuple_size<const T> this way.

If the value member exists, clang still validates that it is an ICE, but
if it does not, then the type is considered to not be tuple-like.

Fixes PR33236

Event Timeline

rnk created this revision.Aug 9 2019, 4:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2019, 4:54 PM
rsmith added a comment.Aug 9 2019, 6:22 PM

Looks good. Can you add a test to test/CXX/drs/dr23xx.cpp with a suitable dr2386: 10 comment to track that we've implemented DR2386 in Clang 10, please?

Jeroen added a subscriber: Jeroen.Aug 9 2019, 11:29 PM

In the reproduction of https://bugs.llvm.org/show_bug.cgi?id=33236 there is explicit mentioning of const T. It would be nice if the test cases for this fix would also have coverage for that.

clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
15

In the reproduction of https://bugs.llvm.org/show_bug.cgi?id=33236 there is explicit mentioning of const T. It would be nice if the test cases for this fix would also have coverage for that.

thakis added a subscriber: thakis.Aug 15 2019, 9:05 AM

rnk, what's the status here?

rnk marked an inline comment as done.Aug 15 2019, 9:10 AM

rnk, what's the status here?

I think I can get this in today, things just got busy here and I forgot about this. I have to add that test.

clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
15

I'll work it into the dr2386 test case.

rnk updated this revision to Diff 215442.Aug 15 2019, 10:52 AM
  • add DR test case
rsmith accepted this revision.Aug 15 2019, 12:14 PM

LGTM with minor adjustments to the test.

clang/test/CXX/drs/dr23xx.cpp
43–57 ↗(On Diff #215442)

Please add a comment

// dr2386: 9

so that the script that generates cxx_dr_status.html knows to mark that issue as done.

It'd be good to also move as much of this test into a namespace dr2386 as possible. (Clearly some parts of it need to be in namespace std, but I'd prefer that those parts be kept as small as possible to isolate this test from others in the same file.)

This revision is now accepted and ready to land.Aug 15 2019, 12:14 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2019, 12:44 PM