Re: [PATCH] Fix PR c++/68948 (wrong code generation due to invalid constructor call)

[Jason Merrill <jason at redhat dot com>]

On 02/05/2016 09:13 AM, Jason Merrill wrote:
> On 02/05/2016 07:54 AM, Patrick Palka wrote:
> > On Thu, 4 Feb 2016, Patrick Palka wrote:
> >
> > > The compiler correctly detects and diagnoses invalid constructor calls
> > > such as C::C () in a non-template context but it fails to do so while
> > > processing a class template.  [ Section 3.4.3.1 of the standard is what
> > > makes these forms of constructor calls illegal -- see
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68948#c9  ]
> > >
> > > In a non-template context this diagnosis would take place in
> > > build_new_method_call, called from finish_call_expr, but while
> > > processing a class template we may exit early out of finish_call_expr
> > > and never call build_new_method_call.
> > >
> > > Thus we never diagnose this invalid constructor call during template
> > > processing.  So then during instantiation of the enclosing template we
> > > call tsubst_baselink on this constructor call, during which the call to
> > > lookup_fnfields returns NULL (because it finds the injected class type C
> > > not the constructor C).  Because the lookup failed, tsubst_baselink
> > > returns error_mark_node and this node persists all the way through to
> > > gimplification where it silently gets discarded.
> > >
> > > This patch fixes this problem by diagnosing these invalid constructor
> > > calls in tsubst_baselink.  Alternatively, we can rewire finish_call_expr
> > > avoid exiting early while processing a template if the call in question
> > > is a constructor call.  I'm not sure which approach is better.  This
> > > approach seems more conservative, since it's just attaching an error
> > > message to an existing error path.
> >
> > And here is the other approach, which rewires finish_call_expr:
>
> I like the second approach better, but you're right that the first is
> more conservative, so let's go with the first for GCC 6 and switch to
> the second for GCC 7.

I'm also applying this patch so that similar issues ICE rather than 
silently generate bad code.

commit f15c5ca5e31d39fb13ef700afeb43aad0c8c7903
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Feb 5 10:34:33 2016 -0500

    	Make issues similar to PR c++/68948 fail loudly.
    
    	* semantics.c (finish_expr_stmt): If expr is error_mark_node,
    	make sure we've seen_error().

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 95c4f19..c9f9db4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -673,6 +673,9 @@ finish_expr_stmt (tree expr)
 
   if (expr != NULL_TREE)
     {
+      /* If we ran into a problem, make sure we complained.  */
+      gcc_assert (expr != error_mark_node || seen_error ());
+
       if (!processing_template_decl)
 	{
 	  if (warn_sequence_point)