Re: cxx-mem-model merge [4 of 9] c-family

[Andrew MacLeod <amacleod at redhat dot com>]

On 11/03/2011 08:20 PM, Joseph S. Myers wrote:
> On Thu, 3 Nov 2011, Andrew MacLeod wrote:
>
> > +   if (VEC_length (tree, params) != n_param)
> > +     {
> > +       error ("Incorrect number of arguments to function %qE", function);
> Diagnostics start with lowercase letters.  The functions such as "error"
> that implicitly use input_location are deprecated - all the new functions
> producing diagnostics should take location_t parameters that they pass to
> error_at.  (If a location isn't readily available in the caller then you
> might make the caller pass input_location - but you still shouldn't
> introduce a new function like this with several calls to plain "error".)

OK.
> > +   type_0 = TREE_TYPE (VEC_index (tree, params, 0));
> > +   if (TREE_CODE (type_0) != POINTER_TYPE)
> > +     {
> > +       error ("argument 0 of %qE must be a pointer type", function);
> Arguments are numbered from 1 in existing diagnostics, not 0.  The
> diagnostics here need to be fixed to match that.
Done.
> > +   /* Check memory model parameters for validity.  */
> > +   for (x = n_param - n_model ; x<  n_param; x++)
> > +     {
> > +       tree p = VEC_index (tree, params, x);
> > +       if (TREE_CODE (p) == INTEGER_CST)
> > +         {
> > + 	  int i = tree_low_cst (p, 1);
> > + 	  if (i<  0 || i>= MEMMODEL_LAST)
> > + 	    {
> > + 	      error ("invalid memory model argument %d of %qE", x, function);
> Given that C and C++ allow variable arguments to the standard type-generic
> macros/functions, it is also presumably OK - only undefined at runtime -
> for a constant memory model argument of the correct type to have a value
> representable in that type that isn't actually a valid memory model.  And
> so this should generate a warning, not an error (ideally generating a trap
> and an informative note that such a trap has been generated - remember
> that the side-effects of the function arguments must be evaluated before
> the trap, in case an argument exits the program, as in
> gcc.c-torture/execute/{call-trap-1.c,va-arg-trap-1.c}).
I don't think I agree.  All the __atomic operations are noexcept, so 
they can't throw.   We currently default unknown runtime values to 
seq-cst for inlined atomics, and presumably the library will also 
implement any unknown value as seq-cst.  There are very explicit 
enumerated types to be used, and I do not think its unreasonable to 
issue an error if someone provides an out of range value which can be 
determined at compile time.

And on a personal note, I will reiterate my annoyance that a 
non-constant is even allowed for this parameter :-)  But I'm over that.  
Mostly. :-)

New patch attached.  I'll work on the updated changelogs tomorrow.  
well, later today :-P

Andrew

Attachment: c-family-2.diff
Description: Text document