Re: Fix up vla, vm and [*] semantics for C

[Mike Stump <mrs at apple dot com>]

On May 12, 2006, at 3:10 PM, Joseph S. Myers wrote:
> > +bool
> > +c_vla_type_p (tree t)
>
> Needs comment.

Fixed.

> > +  /* True when these areguments had [*].  */
>
> Typo, should be "arguments".

Fixed.

> +       if (c_vla_type_p (t1) || c_vla_type_p (t2))
> +           break;
>
> Why is this needed when there's
>
>         if (d1_variable || d2_variable)
>           break;
>
> shortly above which should do the same thing?

The joys of [0] != [*] != [n].  Testing showed that I got it just a  
little bit wrong, after updating it, it now looks like:

@@ -330,6 +341,8 @@ composite_type (tree t1, tree t2)
        d2_variable = (!d2_zero
                       && (TREE_CODE (TYPE_MIN_VALUE (d2)) !=  
INTEGER_CST
                           || TREE_CODE (TYPE_MAX_VALUE (d2)) !=  
INTEGER_CST));
+       d1_variable = d1_variable || (d1_zero && c_vla_type_p (t1));
+       d2_variable = d2_variable || (d2_zero && c_vla_type_p (t2));

        /* Save space: see if the result is identical to one of the  
args.  */
        if (elt == TREE_TYPE (t1) && TYPE_DOMAIN (t1)
@@ -834,6 +847,8 @@ comptypes_internal (tree type1, tree typ
        d2_variable = (!d2_zero
                       && (TREE_CODE (TYPE_MIN_VALUE (d2)) !=  
INTEGER_CST
                           || TREE_CODE (TYPE_MAX_VALUE (d2)) !=  
INTEGER_CST));
+       d1_variable = d1_variable || (d1_zero && c_vla_type_p (t1));
+       d2_variable = d2_variable || (d2_zero && c_vla_type_p (t2));

        if (d1_variable || d2_variable)
          break;

which fixes the bug in my version, and I think addresses the issue  
you raised.  Roughly, [*] is a vla, [0] isn't.  We can intermix the  
features in any combination, and they have to work, I believe I now  
have the code to do this now.

> You use
>
> +      if (was_vm)
> +	c_finish_expr_stmt (expr.value);
>
> to evaluate the argument of typeof.  Will this ensure the argument  
> only
> gets evaluated when the typeof gets evaluated?

Good catch.  No, not that version.  I think I'd rather err on the  
side of no evaluation, instead of regressing code that should have no  
evaluation and adding an extra (wrong) evaluation.  So, to do that, I  
now check for skipping contexts and avoid evaluations in those contexts:

@@ -2155,7 +2156,13 @@ c_parser_typeof_specifier (c_parser *par
          && DECL_C_BIT_FIELD (TREE_OPERAND (expr.value, 1)))
        error ("%<typeof%> applied to a bit-field");
       ret.spec = TREE_TYPE (expr.value);
-      pop_maybe_used (variably_modified_type_p (ret.spec, NULL_TREE));
+      was_vm = variably_modified_type_p (ret.spec, NULL_TREE);
+      /* This should be returned with the type so that when the type
+        is evaluated, this can be evaluated.  For now, we avoid
+        evaluation when the context might.  */
+      if (!skip_evaluation && was_vm)
+       c_finish_expr_stmt (expr.value);
+      pop_maybe_used (was_vm);
     }
   c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)% 
>");
   return ret;

Not ideal, but closer to the semantics we want.  If someone has a  
easy way to arrange  the evaluations, let me know.

> For example, the typeof may not get evaluated inside sizeof, or in  
> a cast in conditionally
> executed code.  sizeof(typeof(VLA)) evaluates the VLA, sizeof(typeof 
> (VM))
> doesn't evaluate the non-VLA VM.

I've added testcases for this:

  if (i != 4)
    return 1;
  __alignof__(__typeof__(t + (i++,0))); /* typeof is not evauluated  
when given a vm inside alignof*/
  if (i != 4)
    return 1;
  sizeof(__typeof__(t + (i++,0)));      /* typeof is not evauluated  
when given a vm inside sizeof*/
  if (i != 4)
    return 1;
  return 0;

I think that fixes up all the important bits.

Ok?

Ok for 4.2?

Attachment: vla-1d.diffs.txt
Description: Text document