diff mbox series

[v2] c++: failure to suppress -Wsizeof-array-div in template [PR114983]

Message ID Zj0j4yfcRH9JBMVl@redhat.com
State New
Headers
Series [v2] c++: failure to suppress -Wsizeof-array-div in template [PR114983] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 warning Patch is already merged

Commit Message

Marek Polacek May 9, 2024, 7:28 p.m. UTC 
  On Thu, May 09, 2024 at 12:51:28PM -0400, Jason Merrill wrote:
> On 5/9/24 12:03, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > -Wsizeof-array-div offers a way to suppress the warning by wrapping
> > the second operand of the division in parens:
> > 
> >    sizeof (samplesBuffer) / (sizeof(unsigned char))
> > 
> > but this doesn't work in a template, because we fail to propagate
> > the suppression bits.  Do it, then.
> > 
> > The finish_parenthesized_expr hunk is not needed because suppress_warning
> > isn't very fine-grained.  But I think it makes sense to be explicit and
> > not rely on OPT_Wparentheses also suppressing OPT_Wsizeof_array_div.
> > 
> > 	PR c++/114983
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* pt.cc (tsubst_expr) <case SIZEOF_EXPR>: Use copy_warning.
> > 	* semantics.cc (finish_parenthesized_expr): Also suppress
> > 	-Wsizeof-array-div.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/warn/Wsizeof-array-div3.C: New test.
> > ---
> >   gcc/cp/pt.cc                                  |  1 +
> >   gcc/cp/semantics.cc                           |  2 ++
> >   .../g++.dg/warn/Wsizeof-array-div3.C          | 27 +++++++++++++++++++
> >   3 files changed, 30 insertions(+)
> >   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C
> > 
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 8787eabb9fd..46cf8a737fb 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -20569,6 +20569,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> >   		TREE_SIDE_EFFECTS (r) = 0;
> >   		TREE_READONLY (r) = 1;
> >   	      }
> > +	    copy_warning (r, t);
> >   	    SET_EXPR_LOCATION (r, EXPR_LOCATION (t));
> 
> copy_warning is defined in terms of locations (as well as TREE_NO_WARNING),
> so copying it before setting the location looks wrong.

Sigh.  Since 'r' had no location, this just set TREE_NO_WARNING, also fixing
this test.  But it's not what I meant to do.

Fixed below, thanks.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
-Wsizeof-array-div offers a way to suppress the warning by wrapping
the second operand of the division in parens:

  sizeof (samplesBuffer) / (sizeof(unsigned char))

but this doesn't work in a template, because we fail to propagate
the suppression bits.  Do it, then.

The finish_parenthesized_expr hunk is not needed because suppress_warning
isn't very fine-grained.  But I think it makes sense to be explicit and
not rely on OPT_Wparentheses also suppressing OPT_Wsizeof_array_div.

	PR c++/114983

gcc/cp/ChangeLog:

	* pt.cc (tsubst_expr) <case SIZEOF_EXPR>: Use copy_warning.
	* semantics.cc (finish_parenthesized_expr): Also suppress
	-Wsizeof-array-div.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Wsizeof-array-div3.C: New test.
---
 gcc/cp/pt.cc                                  |  1 +
 gcc/cp/semantics.cc                           |  2 ++
 .../g++.dg/warn/Wsizeof-array-div3.C          | 27 +++++++++++++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C


base-commit: e02b5683e77c2b4317b23be72e43b6e6cc6c8e5b

Comments

Jason Merrill May 9, 2024, 7:49 p.m. UTC | #1 
On 5/9/24 15:28, Marek Polacek wrote:
> On Thu, May 09, 2024 at 12:51:28PM -0400, Jason Merrill wrote:
>> On 5/9/24 12:03, Marek Polacek wrote:
>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>> index 8787eabb9fd..46cf8a737fb 100644
>>> --- a/gcc/cp/pt.cc
>>> +++ b/gcc/cp/pt.cc
>>> @@ -20569,6 +20569,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>>>    		TREE_SIDE_EFFECTS (r) = 0;
>>>    		TREE_READONLY (r) = 1;
>>>    	      }
>>> +	    copy_warning (r, t);
>>>    	    SET_EXPR_LOCATION (r, EXPR_LOCATION (t));
>>
>> copy_warning is defined in terms of locations (as well as TREE_NO_WARNING),
>> so copying it before setting the location looks wrong.
> 
> Sigh.  Since 'r' had no location, this just set TREE_NO_WARNING, also fixing
> this test.  But it's not what I meant to do.
> 
> Fixed below, thanks.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> -- >8 --
> -Wsizeof-array-div offers a way to suppress the warning by wrapping
> the second operand of the division in parens:
> 
>    sizeof (samplesBuffer) / (sizeof(unsigned char))
> 
> but this doesn't work in a template, because we fail to propagate
> the suppression bits.  Do it, then.
> 
> The finish_parenthesized_expr hunk is not needed because suppress_warning
> isn't very fine-grained.  But I think it makes sense to be explicit and
> not rely on OPT_Wparentheses also suppressing OPT_Wsizeof_array_div.
> 
> 	PR c++/114983
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (tsubst_expr) <case SIZEOF_EXPR>: Use copy_warning.
> 	* semantics.cc (finish_parenthesized_expr): Also suppress
> 	-Wsizeof-array-div.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/warn/Wsizeof-array-div3.C: New test.
> ---
>   gcc/cp/pt.cc                                  |  1 +
>   gcc/cp/semantics.cc                           |  2 ++
>   .../g++.dg/warn/Wsizeof-array-div3.C          | 27 +++++++++++++++++++
>   3 files changed, 30 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 8787eabb9fd..a7d9fcf930e 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -20570,6 +20570,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   		TREE_READONLY (r) = 1;
>   	      }
>   	    SET_EXPR_LOCATION (r, EXPR_LOCATION (t));
> +	    copy_warning (r, t);
>   	  }
>   	RETURN (r);
>         }
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index b8c2bf8771f..71520dcd4fa 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -2306,6 +2306,8 @@ finish_parenthesized_expr (cp_expr expr)
>         /* This inhibits warnings in maybe_warn_unparenthesized_assignment
>   	 and c_common_truthvalue_conversion.  */
>         suppress_warning (STRIP_REFERENCE_REF (*expr), OPT_Wparentheses);
> +      /* And maybe_warn_sizeof_array_div.  */
> +      suppress_warning (STRIP_REFERENCE_REF (*expr), OPT_Wsizeof_array_div);
>       }
>   
>     if (TREE_CODE (expr) == OFFSET_REF
> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C
> new file mode 100644
> index 00000000000..bfd690325e5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C
> @@ -0,0 +1,27 @@
> +// PR c++/114983
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wsizeof-array-div" }
> +
> +using size_t = decltype (sizeof (0));
> +unsigned int samplesBuffer[40];
> +
> +template <typename T>
> +constexpr inline size_t fn1()
> +{
> +  return ((sizeof(samplesBuffer)) / (sizeof(T))); // { dg-bogus "expression does not compute" }
> +}
> +
> +template <typename T>
> +constexpr inline size_t fn2()
> +{
> +  return ((sizeof(samplesBuffer)) / sizeof(T)); // { dg-warning "expression does not compute" }
> +}
> +
> +size_t
> +g ()
> +{
> +  auto sz = sizeof (samplesBuffer) / (sizeof(unsigned char));
> +  sz += fn1<unsigned char>();
> +  sz += fn2<unsigned char>(); // { dg-message "required from here" }
> +  return sz;
> +}
> 
> base-commit: e02b5683e77c2b4317b23be72e43b6e6cc6c8e5b
diff mbox series

Patch

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 8787eabb9fd..a7d9fcf930e 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -20570,6 +20570,7 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		TREE_READONLY (r) = 1;
 	      }
 	    SET_EXPR_LOCATION (r, EXPR_LOCATION (t));
+	    copy_warning (r, t);
 	  }
 	RETURN (r);
       }
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index b8c2bf8771f..71520dcd4fa 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -2306,6 +2306,8 @@  finish_parenthesized_expr (cp_expr expr)
       /* This inhibits warnings in maybe_warn_unparenthesized_assignment
 	 and c_common_truthvalue_conversion.  */
       suppress_warning (STRIP_REFERENCE_REF (*expr), OPT_Wparentheses);
+      /* And maybe_warn_sizeof_array_div.  */
+      suppress_warning (STRIP_REFERENCE_REF (*expr), OPT_Wsizeof_array_div);
     }
 
   if (TREE_CODE (expr) == OFFSET_REF
diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C
new file mode 100644
index 00000000000..bfd690325e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div3.C
@@ -0,0 +1,27 @@ 
+// PR c++/114983
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wsizeof-array-div" }
+
+using size_t = decltype (sizeof (0));
+unsigned int samplesBuffer[40];
+
+template <typename T>
+constexpr inline size_t fn1()
+{
+  return ((sizeof(samplesBuffer)) / (sizeof(T))); // { dg-bogus "expression does not compute" }
+}
+
+template <typename T>
+constexpr inline size_t fn2()
+{
+  return ((sizeof(samplesBuffer)) / sizeof(T)); // { dg-warning "expression does not compute" }
+}
+
+size_t
+g ()
+{
+  auto sz = sizeof (samplesBuffer) / (sizeof(unsigned char));
+  sz += fn1<unsigned char>();
+  sz += fn2<unsigned char>(); // { dg-message "required from here" }
+  return sz;
+}