Re: Patch for C++ uninitialized pointer-to-member value

[Jason Merrill <jason at redhat dot com>]

>>>>> "Alexandre" == Alexandre Oliva <aoliva@redhat.com> writes:

Thanks for following up on this.

> @@ -290,7 +290,12 @@ convert_to_pointer_force (type, expr)
   
>    if (integer_zerop (expr))
>      {
> -      expr = build_int_2 (0, 0);
> +      if (TYPE_PTRMEM_P (type))
> +	/* Under the new ABI, a NULL pointer-to-member is represented
> +	   by -1, not by zero.  */
> +	expr = build_int_2 (-1, -1);
> +      else
> +	expr = build_int_2 (0, 0);
>        TREE_TYPE (expr) = type;
>        return expr;
>      }

Rather than duplicate this test, I'd rather let this case fall through to
cp_convert_to_pointer.

> @@ -7592,7 +7592,10 @@ obscure_complex_init (decl, init)
>  				 NULL_TREE);
>    else
>  #endif
> -    DECL_INITIAL (decl) = error_mark_node;
> +    {
> +      if (zero_init_p (TREE_TYPE (decl)))
> +	DECL_INITIAL (decl) = error_mark_node;
> +    }
 
So what happens to DECL_INITIAL if !zero_init_p?

> @@ -7832,14 +7835,44 @@ check_initializer (decl, init)
>        my_friendly_assert (init != NULL_TREE, 149);
>        init = NULL_TREE;
>      }
> -  else if (!DECL_EXTERNAL (decl) && TREE_CODE (type) == REFERENCE_TYPE)
> +  else if (! DECL_EXTERNAL (decl) && TREE_CODE (type) == REFERENCE_TYPE)

FWIW, I think the official style is to omit this space.  I disagree, but
there it is.

>      {
>        init = grok_reference_init (decl, type, init);
>        if (init)
>  	init = obscure_complex_init (decl, init);
>      }
> +  else if (! DECL_EXTERNAL (decl) && ! zero_init_p (type))
> +    {
> +      tree static_init = NULL;
> +
> +      if (CLASS_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
> +	static_init = build (CONSTRUCTOR, NULL_TREE, NULL_TREE, NULL_TREE);
> +      else if (TYPE_PTRMEM_P (type))
> +	static_init = integer_zero_node;
> +      else
> +	abort ();
> +
> +      static_init = digest_init (type, static_init, 0);
> +
> +      if (TREE_CODE (static_init) != TREE_VEC)
> +	{
> +	  int needs_constructing = TYPE_NEEDS_CONSTRUCTING (type);
> +
> +	  TYPE_NEEDS_CONSTRUCTING (type) = 0;
> +
> +	  static_init = store_init_value (decl, static_init);
> +	  if (static_init)
> +	    abort ();
> +
> +	  TYPE_NEEDS_CONSTRUCTING (type) = needs_constructing;
> +	}
> +
> +      if (init)
> +	goto process_init;
> +    }
>    else if (init)
>      {
> +    process_init:
>        if (TYPE_HAS_CONSTRUCTOR (type) || TYPE_NEEDS_CONSTRUCTING (type))
>  	{
>  	  if (TREE_CODE (type) == ARRAY_TYPE)

> @@ -732,6 +732,18 @@ process_init_constructor (type, init, el
>  		next1 = build (CONSTRUCTOR, NULL_TREE, NULL_TREE, NULL_TREE);
>  	      next1 = digest_init (TREE_TYPE (type), next1, 0);
>  	    }
> +	  else if (! zero_init_p (TREE_TYPE (type)))
> +	    {
> +	      if (CLASS_TYPE_P (TREE_TYPE (type))
> +		  || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
> +		next1 = build (CONSTRUCTOR, NULL_TREE, NULL_TREE, NULL_TREE);
> +	      else if (TYPE_PTRMEM_P (TREE_TYPE (type)))
> +		next1 = integer_zero_node;
> +	      else
> +		abort ();
> +	      
> +	      next1 = digest_init (TREE_TYPE (type), next1, 0);
> +	    }
>  	  else
>  	    /* The default zero-initialization is fine for us; don't
>  	       add anything to the CONSTRUCTOR.  */
> @@ -848,9 +860,23 @@ process_init_constructor (type, init, el
>  	          && (!init || TREE_HAS_CONSTRUCTOR (init)))
>  		warning ("missing initializer for member `%D'", field);
 
> -	      /* The default zero-initialization is fine for us; don't
> -		 add anything to the CONSTRUCTOR.  */
> -	      continue;
> +	      if (! zero_init_p (TREE_TYPE (field)))
> +		{
> +		  if (CLASS_TYPE_P (TREE_TYPE (field))
> +		      || TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE)
> +		    next1 = build (CONSTRUCTOR, NULL_TREE,
> +				   NULL_TREE, NULL_TREE);
> +		  else if (TYPE_PTRMEM_P (TREE_TYPE (field)))
> +		    next1 = integer_zero_node;
> +		  else
> +		    abort ();
> +	      
> +		  next1 = digest_init (TREE_TYPE (field), next1, 0);
> +		}
> +	      else
> +		/* The default zero-initialization is fine for us; don't
> +		   add anything to the CONSTRUCTOR.  */
> +		continue;
>  	    }

Three copies of this code is too many.  Four, if you count
build_default_init.  Please factor it out.

Thanks,
Jason