diff mbox series

[C++] PR 80956 ("[7/8 Regression] ICE with abstract class vector")

Message ID e2c2abf8-5c9d-5bf9-0851-9eedcf2a4aa8@oracle.com
State New
Headers show
Series [C++] PR 80956 ("[7/8 Regression] ICE with abstract class vector") | expand

Commit Message

Paolo Carlini April 5, 2018, 12:27 p.m. UTC
Hi,

the main issue is already fixed in trunk but we still ICE on the reduced 
testcase attached by Jakub which has a broken std::initializer_list 
missing the definition. I think we can handle this case similarly to the 
existing check in do_pushtag, which would be also consistent with the 
plain error we give for, eg:

namespace std { template <class> class initializer_list; }

template class std::initializer_list<int>;

However, we still have the option of issuing a fatal_error, like we do 
in finish_struct.

In any case, I'm tweaking for consistency do_pushtag about %< and %>.

Tested x86_64-linux.

Thanks, Paolo.

///////////////////
/cp
2018-04-05  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/80956
	* call.c (convert_like_real): Fail gracefully for a broken
	std::initializer_list, missing a definition.

	* name-lookup.c (do_pushtag): Tweak message, use %< and %>.

/testsuite
2018-04-05  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/80956
	* g++.dg/cpp0x/initlist100.C: New.
	* g++.dg/cpp0x/initlist101.C: Likewise.

Comments

Jason Merrill April 5, 2018, 1:56 p.m. UTC | #1
On Thu, Apr 5, 2018 at 8:27 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> the main issue is already fixed in trunk but we still ICE on the reduced
> testcase attached by Jakub which has a broken std::initializer_list missing
> the definition. I think we can handle this case similarly to the existing
> check in do_pushtag, which would be also consistent with the plain error we
> give for, eg:
>
> namespace std { template <class> class initializer_list; }
>
> template class std::initializer_list<int>;
>
> However, we still have the option of issuing a fatal_error, like we do in
> finish_struct.

How about using complete_type_or_maybe_complain instead of a custom error?

Jason
Paolo Carlini April 5, 2018, 2:07 p.m. UTC | #2
Hi,

On 05/04/2018 15:56, Jason Merrill wrote:
> On Thu, Apr 5, 2018 at 8:27 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> the main issue is already fixed in trunk but we still ICE on the reduced
>> testcase attached by Jakub which has a broken std::initializer_list missing
>> the definition. I think we can handle this case similarly to the existing
>> check in do_pushtag, which would be also consistent with the plain error we
>> give for, eg:
>>
>> namespace std { template <class> class initializer_list; }
>>
>> template class std::initializer_list<int>;
>>
>> However, we still have the option of issuing a fatal_error, like we do in
>> finish_struct.
> How about using complete_type_or_maybe_complain instead of a custom error?
Yes, I was about to send a message about that option, I already had it 
in the audit trail, tested too, then started fiddling with fatal_errors 
and forgot to mention it ;) Anyway, would be the below.

Thanks,
Paolo.

/////////////////////
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 259124)
+++ cp/call.c	(working copy)
@@ -6880,8 +6880,12 @@ convert_like_real (conversion *convs, tree expr, t
 	if (array == error_mark_node)
 	  return error_mark_node;
 
-	/* Build up the initializer_list object.  */
-	totype = complete_type (totype);
+	/* Build up the initializer_list object.  Note: fail gracefully
+	   if the object cannot be completed because, for example, no
+	   definition is provided.  */
+	totype = complete_type_or_maybe_complain (totype, NULL_TREE, complain);
+	if (!totype)
+	  return error_mark_node;
 	field = next_initializable_field (TYPE_FIELDS (totype));
 	CONSTRUCTOR_APPEND_ELT (vec, field, array);
 	field = next_initializable_field (DECL_CHAIN (field));
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 259124)
+++ cp/name-lookup.c	(working copy)
@@ -6476,8 +6476,8 @@ do_pushtag (tree name, tree type, tag_scope scope)
 	      && init_list_identifier == DECL_NAME (TYPE_NAME (type))
 	      && !CLASSTYPE_TEMPLATE_INFO (type))
 	    {
-	      error ("declaration of std::initializer_list does not match "
-		     "#include <initializer_list>, isn't a template");
+	      error ("declaration of %<std::initializer_list%> does not match "
+		     "%<#include <initializer_list>%>, isn't a template");
 	      return error_mark_node;
 	    }
 	}
Index: testsuite/g++.dg/cpp0x/initlist100.C
===================================================================
--- testsuite/g++.dg/cpp0x/initlist100.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/initlist100.C	(working copy)
@@ -0,0 +1,10 @@
+// PR c++/80956
+// { dg-do compile { target c++11 } }
+
+namespace std {
+template <class> class initializer_list;  // { dg-message "declaration" }
+}
+
+template <typename T> struct B { B (std::initializer_list<T>); };
+struct C { virtual int foo (); };
+struct D : C {} d { B<C> { D {} } };  // { dg-error "incomplete|no matching" }
Index: testsuite/g++.dg/cpp0x/initlist101.C
===================================================================
--- testsuite/g++.dg/cpp0x/initlist101.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/initlist101.C	(working copy)
@@ -0,0 +1,8 @@
+// PR c++/80956
+// { dg-do compile { target c++11 } }
+
+#include <initializer_list>
+
+template <typename T> struct B { B (std::initializer_list<T>); };
+struct C { virtual int foo (); };
+struct D : C {} d { B<C> { D {} } };  // { dg-error "no matching" }
Jason Merrill April 5, 2018, 2:35 p.m. UTC | #3
On Thu, Apr 5, 2018 at 10:07 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 05/04/2018 15:56, Jason Merrill wrote:
>>
>> On Thu, Apr 5, 2018 at 8:27 AM, Paolo Carlini <paolo.carlini@oracle.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> the main issue is already fixed in trunk but we still ICE on the reduced
>>> testcase attached by Jakub which has a broken std::initializer_list
>>> missing
>>> the definition. I think we can handle this case similarly to the existing
>>> check in do_pushtag, which would be also consistent with the plain error
>>> we
>>> give for, eg:
>>>
>>> namespace std { template <class> class initializer_list; }
>>>
>>> template class std::initializer_list<int>;
>>>
>>> However, we still have the option of issuing a fatal_error, like we do in
>>> finish_struct.
>>
>> How about using complete_type_or_maybe_complain instead of a custom error?
>
> Yes, I was about to send a message about that option, I already had it in
> the audit trail, tested too, then started fiddling with fatal_errors and
> forgot to mention it ;) Anyway, would be the below.

OK, thanks.

Jason
diff mbox series

Patch

Index: cp/call.c
===================================================================
--- cp/call.c	(revision 259124)
+++ cp/call.c	(working copy)
@@ -6880,8 +6880,19 @@  convert_like_real (conversion *convs, tree expr, t
 	if (array == error_mark_node)
 	  return error_mark_node;
 
-	/* Build up the initializer_list object.  */
+	/* Build up the initializer_list object.  Note: fail gracefully
+	   if the object cannot be completed because, for example, no
+	   definition is provided.  */
 	totype = complete_type (totype);
+	if (!COMPLETE_TYPE_P (totype))
+	  {
+	    if (complain & tf_error)
+	      error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (totype)),
+			"declaration of %<std::initializer_list%> does not "
+			"match %<#include <initializer_list>%>, cannot be "
+			"completed");
+	    return error_mark_node;
+	  }
 	field = next_initializable_field (TYPE_FIELDS (totype));
 	CONSTRUCTOR_APPEND_ELT (vec, field, array);
 	field = next_initializable_field (DECL_CHAIN (field));
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 259124)
+++ cp/name-lookup.c	(working copy)
@@ -6476,8 +6476,8 @@  do_pushtag (tree name, tree type, tag_scope scope)
 	      && init_list_identifier == DECL_NAME (TYPE_NAME (type))
 	      && !CLASSTYPE_TEMPLATE_INFO (type))
 	    {
-	      error ("declaration of std::initializer_list does not match "
-		     "#include <initializer_list>, isn't a template");
+	      error ("declaration of %<std::initializer_list%> does not match "
+		     "%<#include <initializer_list>%>, isn't a template");
 	      return error_mark_node;
 	    }
 	}
Index: testsuite/g++.dg/cpp0x/initlist100.C
===================================================================
--- testsuite/g++.dg/cpp0x/initlist100.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/initlist100.C	(working copy)
@@ -0,0 +1,10 @@ 
+// PR c++/80956
+// { dg-do compile { target c++11 } }
+
+namespace std {
+template <class> class initializer_list;  // { dg-error "declaration of .*std::initializer_list.* does not match" }
+}
+
+template <typename T> struct B { B (std::initializer_list<T>); };
+struct C { virtual int foo (); };
+struct D : C {} d { B<C> { D {} } };  // { dg-error "no matching" }
Index: testsuite/g++.dg/cpp0x/initlist101.C
===================================================================
--- testsuite/g++.dg/cpp0x/initlist101.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/initlist101.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/80956
+// { dg-do compile { target c++11 } }
+
+#include <initializer_list>
+
+template <typename T> struct B { B (std::initializer_list<T>); };
+struct C { virtual int foo (); };
+struct D : C {} d { B<C> { D {} } };  // { dg-error "no matching" }