[v3 of PATCH 15/14] Use fold_for_warn in get_atomic_generic_size

[David Malcolm]

On Tue, 2017-12-19 at 23:22 -0500, Jason Merrill wrote:
> On Tue, Dec 19, 2017 at 7:53 PM, David Malcolm <dmalc...@redhat.com>
> wrote:
> > On Tue, 2017-12-19 at 15:35 -0500, Jason Merrill wrote:
> > > On Sat, Dec 16, 2017 at 8:12 PM, David Malcolm <dmalcolm@redhat.c
> > > om>
> > > wrote:
> > > > I rebased the v2 patchkit; here's an extra patch to fix an
> > > > issue
> > > > with it uncovered by a recently-added testcase (in r254990).
> > > > 
> > > > With the patch kit, but without this patch, g++'s
> > > >   c-c++-common/pr83059.c
> > > > fails to emit the "invalid memory model argument 6" warning.
> > > > 
> > > > Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu, as
> > > > part of the kit.
> > > > 
> > > > Is this OK for trunk, assuming the rest of the kit is approved?
> > > > 
> > > > gcc/c-family/ChangeLog:
> > > >         * c-common.c (get_atomic_generic_size): Call
> > > > fold_for_warn
> > > > on the
> > > >         params before checking for INTEGER_CST.
> > > > ---
> > > >  gcc/c-family/c-common.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > > > index 3438b87..ab03b7d 100644
> > > > --- a/gcc/c-family/c-common.c
> > > > +++ b/gcc/c-family/c-common.c
> > > > @@ -6720,7 +6720,7 @@ get_atomic_generic_size (location_t loc,
> > > > tree
> > > > function,
> > > >    /* Check memory model parameters for validity.  */
> > > >    for (x = n_param - n_model ; x < n_param; x++)
> > > >      {
> > > > -      tree p = (*params)[x];
> > > > +      tree p = fold_for_warn ((*params)[x]);
> > > >        if (TREE_CODE (p) == INTEGER_CST)
> > > >          {
> > > >           /* memmodel_base masks the low 16 bits, thus ignore
> > > > any
> > > > bits above
> > > 
> > > Let's check the error case before we call fold_for_warn.
> > > 
> > > Jason
> > 
> > Do you mean like this?  (bootstrapped; regrtest in progress)
> 
> Ah, no, sorry I wasn't clear.  I meant to reorder the if/else there,
> so we check for INTEGER_TYPE first and potentially give an error,
> then
> fold, and then potentially warn.
> 
> Jason

Aha - thanks!

Here's an updated version of the patch.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu, as
part of the kit.

OK for trunk once the rest of the kit is approved?

gcc/c-family/ChangeLog:
        * c-common.c (get_atomic_generic_size): Perform the test for
        integral type before the range test for any integer constant,
        fixing indentation of braces.  Call fold_for_warn before
        testing for an INTEGER_CST.
---
 gcc/c-family/c-common.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3438b87..7cc749b 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -6721,8 +6721,15 @@ get_atomic_generic_size (location_t loc, tree function,
   for (x = n_param - n_model ; x < n_param; x++)
     {
       tree p = (*params)[x];
+      if (!INTEGRAL_TYPE_P (TREE_TYPE (p)))
+       {
+         error_at (loc, "non-integer memory model argument %d of %qE", x + 1,
+                   function);
+         return 0;
+       }
+      p = fold_for_warn (p);
       if (TREE_CODE (p) == INTEGER_CST)
-        {
+       {
          /* memmodel_base masks the low 16 bits, thus ignore any bits above
             it by using TREE_INT_CST_LOW instead of tree_to_*hwi.  Those high
             bits will be checked later during expansion in target specific
@@ -6732,14 +6739,7 @@ get_atomic_generic_size (location_t loc, tree function,
                        "invalid memory model argument %d of %qE", x + 1,
                        function);
        }
-      else
-       if (!INTEGRAL_TYPE_P (TREE_TYPE (p)))
-         {
-           error_at (loc, "non-integer memory model argument %d of %qE", x + 1,
-                  function);
-           return 0;
-         }
-      }
+    }
 
   return size_0;
 }
-- 
1.8.5.3