diff mbox

PATCH to shorten_compare -Wtype-limits handling

Message ID 564F419B.4080507@redhat.com
State New
Headers show

Commit Message

Jason Merrill Nov. 20, 2015, 3:51 p.m. UTC 
On 11/19/2015 05:16 PM, Jason Merrill wrote:
> On 11/19/2015 02:44 PM, Martin Sebor wrote:
>> On 11/18/2015 09:26 PM, Jason Merrill wrote:
>>> The rs6000 target was hitting a bootstrap failure due to
>>> -Werror=type-limits.  Since warn_tautological_cmp and other warnings
>>> avoid warning if one of the operands comes from a macro, I thought it
>>> would make sense to do that here as well.
>>
>> The also disables the warning for functions that are shadowed by
>> macros such as C atomic_load et al. For example, in the program
>> below. Is that an acceptable compromise or is there a way to avoid
>> this?
>
> I think it's an acceptable compromise, but see below.
>
>> At the same time, the change doesn't suppress the warning in other
>> cases where I would have expected it to suppress it based on your
>> description. For instance here:
>>
>>    unsigned short bar (unsigned short x)
>>    {
>>    #define X x
>>
>>      if (x > 0xffff)
>
> Yes, this is missed because the front end doesn't remember the location
> of the use of x; that's one of many location tracking issues.  David
> Malcolm is working on this stuff.
>
>> I noticed there is code elsewhere in c-common.c that avoids
>> issuing the same warning for system headers (that's the code
>> that responsible for issuing the warning for the second test
>> case above).
>
> Hmm, it looks like using expansion_point_if_in_system_header might avoid
> the first issue you mention.

Thus.

Comments

Martin Sebor Nov. 20, 2015, 4:10 p.m. UTC | #1 
>> Hmm, it looks like using expansion_point_if_in_system_header might avoid
>> the first issue you mention.
>
> Thus.

Great, thanks! (I'll have to remember the trick for my own use!)

Martin
Manuel López-Ibáñez Nov. 20, 2015, 5:19 p.m. UTC | #2 
On 20 November 2015 at 16:10, Martin Sebor <msebor@gmail.com> wrote:
>>> Hmm, it looks like using expansion_point_if_in_system_header might avoid
>>> the first issue you mention.
>>
>>
>> Thus.
>
>
> Great, thanks! (I'll have to remember the trick for my own use!)

I added this to  https://gcc.gnu.org/wiki/DiagnosticsGuidelines under
"Locations" for future reference. I hope others would do the same in
the future, so the info is kept up-to-date.

Cheers,

Manuel.
diff mbox

Patch

commit fb71bf4de520cc4bd11eefb57e50748b4679996f
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Nov 19 15:21:47 2015 -0500

    	* c-common.c (shorten_compare): But look through macros from
    	system headers.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 068a0bc..fe0a235 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -4651,8 +4651,10 @@  shorten_compare (location_t loc, tree *op0_ptr, tree *op1_ptr,
 	}
 
       if (TREE_CODE (primop0) != INTEGER_CST
-	  /* Don't warn if it's from a macro.  */
-	  && !from_macro_expansion_at (EXPR_LOCATION (primop0)))
+	  /* Don't warn if it's from a (non-system) macro.  */
+	  && !(from_macro_expansion_at
+	       (expansion_point_location_if_in_system_header
+		(EXPR_LOCATION (primop0)))))
 	{
 	  if (val == truthvalue_false_node)
 	    warning_at (loc, OPT_Wtype_limits,
diff --git a/gcc/testsuite/gcc.dg/Wtype-limits2.c b/gcc/testsuite/gcc.dg/Wtype-limits2.c
new file mode 100644
index 0000000..92151aa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wtype-limits2.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wtype-limits" } */
+/* { dg-require-effective-target sync_char_short } */
+
+#include <stdatomic.h>
+
+unsigned foo (unsigned char *x)
+{
+  if (atomic_load (x) > 1000) /* { dg-warning "comparison is always false due to limited range of data type" } */
+    return 0;
+  return 1;
+}