This is an archive of the discontinued LLVM Phabricator instance.

[SemaCXX] Fix ICE with structure bindings to members of template
ClosedPublic

Authored by ddiproietto on Jan 20 2019, 1:32 AM.

Details

Summary

Trying to use structure binding with a structure that doesn't implement
std::tuple_size, should unpack the data members. When the struct is a
template though, clang might hit an assertion (if the type has not been
completed before), because CXXRecordDecl::DefinitionData is nullptr.

This commit fixes the problem by completing the type while trying to
decompose the structured binding.

The ICE happens in real world code, for example, when trying to iterate
a protobuf generated map with a range-based for loop and structure
bindings (because google::protobuf::MapPair is a template and doesn't
support std::tuple_size).

Reported-by: nicholas.sun@nlsun.com

Diff Detail

Repository
rL LLVM

Event Timeline

ddiproietto created this revision.Jan 20 2019, 1:32 AM

Yeah, seems like we don't instantiate pair<int, int> at all in that specific case. LGTM but you might want to wait for someone else to confirm.

rsmith accepted this revision.Jan 26 2019, 3:00 PM

LGTM, thanks.

This revision is now accepted and ready to land.Jan 26 2019, 3:00 PM

Sorry, I don't have commit access, can someone push this, please?

Also, should this be backported to 8.x? I'm actually interested in a backport up to 5.x, if possible. If not possible, that's fine too. Thanks!

Cheers,

Daniele

This revision was automatically updated to reflect the committed changes.

Backporting to 8.0.0 is possibly possible, you'll need to ping Hans. I don't know about previous versions though.