[PATCH] D56974: [SemaCXX] Fix ICE with structure bindings to members of template

2019-01-20 Thread Daniele Di Proietto via Phabricator via cfe-commits
ddiproietto created this revision.
ddiproietto added a reviewer: clang.
ddiproietto added a project: clang.
Herald added a subscriber: cfe-commits.

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@nlsun.com


Repository:
  rC Clang

https://reviews.llvm.org/D56974

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/cxx1z-decomposition.cpp


Index: clang/test/SemaCXX/cxx1z-decomposition.cpp
===
--- clang/test/SemaCXX/cxx1z-decomposition.cpp
+++ clang/test/SemaCXX/cxx1z-decomposition.cpp
@@ -81,4 +81,21 @@
   void f() { static auto [a] = *this; } // expected-error {{cannot be declared 
'static'}}
 };
 
+namespace instantiate_template {
+
+template 
+struct pair {
+  T1 a;
+  T2 b;
+};
+
+const pair &f1();
+
+int f2() {
+  const auto &[a, b] = f1();
+  return a + b;
+}
+
+} // namespace instantiate_template
+
 // FIXME: by-value array copies
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1300,6 +1300,10 @@
 static bool checkMemberDecomposition(Sema &S, ArrayRef Bindings,
  ValueDecl *Src, QualType DecompType,
  const CXXRecordDecl *OrigRD) {
+  if (S.RequireCompleteType(Src->getLocation(), DecompType,
+diag::err_incomplete_type))
+return true;
+
   CXXCastPath BasePath;
   DeclAccessPair BasePair =
   findDecomposableBaseClass(S, Src->getLocation(), OrigRD, BasePath);


Index: clang/test/SemaCXX/cxx1z-decomposition.cpp
===
--- clang/test/SemaCXX/cxx1z-decomposition.cpp
+++ clang/test/SemaCXX/cxx1z-decomposition.cpp
@@ -81,4 +81,21 @@
   void f() { static auto [a] = *this; } // expected-error {{cannot be declared 'static'}}
 };
 
+namespace instantiate_template {
+
+template 
+struct pair {
+  T1 a;
+  T2 b;
+};
+
+const pair &f1();
+
+int f2() {
+  const auto &[a, b] = f1();
+  return a + b;
+}
+
+} // namespace instantiate_template
+
 // FIXME: by-value array copies
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1300,6 +1300,10 @@
 static bool checkMemberDecomposition(Sema &S, ArrayRef Bindings,
  ValueDecl *Src, QualType DecompType,
  const CXXRecordDecl *OrigRD) {
+  if (S.RequireCompleteType(Src->getLocation(), DecompType,
+diag::err_incomplete_type))
+return true;
+
   CXXCastPath BasePath;
   DeclAccessPair BasePair =
   findDecomposableBaseClass(S, Src->getLocation(), OrigRD, BasePath);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D56974: [SemaCXX] Fix ICE with structure bindings to members of template

2019-01-27 Thread Daniele Di Proietto via Phabricator via cfe-commits
ddiproietto added a comment.

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56974/new/

https://reviews.llvm.org/D56974



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits