Re: c++/7266 [Candidate fix]

[Zack Weinberg <zack at codesourcery dot com>]

PR c++/7266 reports a crash on the ill-formed

template <class A> struct B {typedef A::C::D E;};

grokdeclarator is called with this declarator:

 <scope_ref
    complexity 1
    arg 0 (null)
    arg 1 <identifier_node D>>

We eventually get to this bit (please forgive the length of the
extract):

	case SCOPE_REF:
	  {
	    /* We have converted type names to NULL_TREE if the
	       name was bogus, or to a _TYPE node, if not.

	       The variable CTYPE holds the type we will ultimately
	       resolve to.  The code here just needs to build
	       up appropriate member types.  */
	    tree sname = TREE_OPERAND (declarator, 1);
	    tree t;

	    /* Destructors can have their visibilities changed as well.  */
	    if (TREE_CODE (sname) == BIT_NOT_EXPR)
	      sname = TREE_OPERAND (sname, 0);

	    if (TREE_COMPLEXITY (declarator) == 0)
	      /* This needs to be here, in case we are called
		 multiple times.  */ ;
	    else if (TREE_COMPLEXITY (declarator) == -1)
	      /* Namespace member.  */
	      pop_decl_namespace ();
	    else if (friendp && (TREE_COMPLEXITY (declarator) < 2))
	      /* Don't fall out into global scope. Hides real bug? --eichin */ ;
	    else if (! IS_AGGR_TYPE_CODE
***		     (TREE_CODE (TREE_OPERAND (declarator, 0))))
	      ;
	    else if (TREE_COMPLEXITY (declarator) == current_class_depth)
	      {
		/* Resolve any TYPENAME_TYPEs from the decl-specifier-seq
		   that refer to ctype.  They couldn't be resolved earlier
		   because we hadn't pushed into the class yet.
		   Example: resolve 'B<T>::type' in
		   'B<typename B<T>::type> B<T>::f () { }'.  */
		if (current_template_parms
		    && uses_template_parms (type)
		    && uses_template_parms (current_class_type))
		  {
		    tree args = current_template_args ();
		    type = tsubst (type, args, tf_error | tf_warning,
				   NULL_TREE);
		  }

		/* This pop_nested_class corresponds to the
                   push_nested_class used to push into class scope for
                   parsing the argument list of a function decl, in
                   qualified_id.  */
		pop_nested_class ();
		TREE_COMPLEXITY (declarator) = current_class_depth;
	      }
	    else
	      abort ();

@@@	    if (TREE_OPERAND (declarator, 0) == NULL_TREE)
	      {
		/* We had a reference to a global decl, or
		   perhaps we were given a non-aggregate typedef,
		   in which case we cleared this out, and should just
		   keep going as though it wasn't there.  */
		declarator = sname;
		continue;
	      }

The crash happens at the starred line.  TREE_OPERAND (declarator, 0)
is NULL, so attempting to take TREE_CODE of it naturally segfaults.
Note the block below marked with at-signs.  Moving it above the series
of tests on TREE_COMPLEXITY makes the crash go away.  Furthermore,
what that block does to a SCOPE_REF with null first operand makes
some sense for this situation.  So that's a candidate fix for the bug.

However, I'm not confident that the rest of the code fragment here is
correct.  What's a test of IS_AGGR_TYPE_CODE doing in the middle of
checks of the TREE_COMPLEXITY of the declarator, anyway?
current_class_depth is 1; should we have had a chance to execute the
block for TREE_COMPLEXITY (declarator) == current_class_depth before
we "just keep going as though it wasn't there"?  Or the other way
around?  Why does that block set TREE_COMPLEXITY (declarator) to the
value it presumably already has?

C++ folks, thoughts would be appreciated.

zw