diff mbox

C PATCH to detect clashing attributes (PR c/81544)

Message ID 20170725153831.GG3397@redhat.com
State New
Headers show

Commit Message

Marek Polacek July 25, 2017, 3:38 p.m. UTC 
PR c/81544 complaints that we aren't detecting clashing noreturn /
warn_unused_result attributes so this patch adds that checking.  Martin
plans to do more systematic checking in this area but meanwhile we
might want to go with this.

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

2017-07-25  Marek Polacek  <polacek@redhat.com>

	PR c/81544
	* c-common.h (diagnose_mismatched_attributes): Update decl.
	* c-warn.c (diagnose_mismatched_attributes): Add OLDTYPE and
	NEWTYPE parameters.  Diagnose clashing noreturn / warn_unused_result
	attributes.

	* c-decl.c (diagnose_mismatched_decls): Pass OLDTYPE and NEWTYPE down
	to diagnose_mismatched_attributes.

	* decl.c (duplicate_decls): Pass the types of OLDDECL and NEWDECL down
	to diagnose_mismatched_attributes.

	* c-c++-common/attributes-4.c: New test.


	Marek

Comments

Joseph Myers July 31, 2017, 10:33 p.m. UTC | #1 
On Tue, 25 Jul 2017, Marek Polacek wrote:

> PR c/81544 complaints that we aren't detecting clashing noreturn /
> warn_unused_result attributes so this patch adds that checking.  Martin
> plans to do more systematic checking in this area but meanwhile we
> might want to go with this.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

I'd expect exactly the same cases to be diagnosed for the two attributes 
on the same declaration, in either order, whether or not inside a single 
__attribute__, as are diagnosed when multiple declarations are involved 
(this applies to all such cases of invalid attribute combinations, not 
just this one).  Whether or not this patch achieves this, the testcase 
doesn't seem to cover the case of a single declaration with both 
attributes (and I don't see an existing such test in c-c++-common or 
gcc.dg either).
diff mbox

Patch

diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index a29f1ade25d..7c4b2a0e108 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -1537,7 +1537,7 @@  extern void maybe_warn_unused_local_typedefs (void);
 extern void maybe_warn_bool_compare (location_t, enum tree_code, tree, tree);
 extern bool maybe_warn_shift_overflow (location_t, tree, tree);
 extern void warn_duplicated_cond_add_or_warn (location_t, tree, vec<tree> **);
-extern bool diagnose_mismatched_attributes (tree, tree);
+extern bool diagnose_mismatched_attributes (tree, tree, tree, tree);
 extern tree do_warn_duplicated_branches_r (tree *, int *, void *);
 extern void warn_for_multistatement_macros (location_t, location_t,
 					    location_t, enum rid);
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index a8b38c1b98d..d3c62a5ad9e 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -2125,7 +2125,8 @@  warn_duplicated_cond_add_or_warn (location_t loc, tree cond, vec<tree> **chain)
    attributes, such as always_inline vs. noinline.  */
 
 bool
-diagnose_mismatched_attributes (tree olddecl, tree newdecl)
+diagnose_mismatched_attributes (tree olddecl, tree newdecl, tree oldtype,
+				tree newtype)
 {
   bool warned = false;
 
@@ -2172,6 +2173,17 @@  diagnose_mismatched_attributes (tree olddecl, tree newdecl)
     warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute "
 		       "%qs follows declaration with attribute %qs",
 		       newdecl, "hot", "cold");
+  else if (lookup_attribute ("noreturn", DECL_ATTRIBUTES (newdecl))
+	   && lookup_attribute ("warn_unused_result",
+				TYPE_ATTRIBUTES (oldtype)))
+    warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute "
+		       "%qs follows declaration with attribute %qs",
+		       newdecl, "noreturn", "warn_unused_result");
+  else if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (newtype))
+	   && lookup_attribute ("noreturn", DECL_ATTRIBUTES (olddecl)))
+    warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute "
+		       "%qs follows declaration with attribute %qs",
+		       newdecl, "warn_unused_result", "noreturn");
   return warned;
 }
 
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index 12fbc18bb94..e995606499f 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2234,7 +2234,8 @@  diagnose_mismatched_decls (tree newdecl, tree olddecl,
     }
 
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
-    warned |= diagnose_mismatched_attributes (olddecl, newdecl);
+    warned |= diagnose_mismatched_attributes (olddecl, newdecl, oldtype,
+					      newtype);
   else /* PARM_DECL, VAR_DECL */
     {
       /* Redeclaration of a parameter is a constraint violation (this is
diff --git gcc/cp/decl.c gcc/cp/decl.c
index d98fab370d7..4a67e57c675 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -1397,7 +1397,9 @@  duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
   if (DECL_P (olddecl)
       && TREE_CODE (newdecl) == FUNCTION_DECL
       && TREE_CODE (olddecl) == FUNCTION_DECL
-      && diagnose_mismatched_attributes (olddecl, newdecl))
+      && diagnose_mismatched_attributes (olddecl, newdecl,
+					 TREE_TYPE (olddecl),
+					 TREE_TYPE (newdecl)))
     {
       if (DECL_INITIAL (olddecl))
 	inform (DECL_SOURCE_LOCATION (olddecl),
diff --git gcc/testsuite/c-c++-common/attributes-4.c gcc/testsuite/c-c++-common/attributes-4.c
index e69de29bb2d..2f8b9676ecd 100644
--- gcc/testsuite/c-c++-common/attributes-4.c
+++ gcc/testsuite/c-c++-common/attributes-4.c
@@ -0,0 +1,8 @@ 
+/* PR c/81544 */
+/* { dg-do compile } */
+
+int __attribute__ ((noreturn)) foo (void); /* { dg-message "previous declaration" } */
+int __attribute__ ((warn_unused_result)) foo (void); /* { dg-warning "attribute .warn_unused_result. follows declaration with attribute .noreturn." } */
+
+int __attribute__ ((warn_unused_result)) foo2 (void); /* { dg-message "previous declaration" } */
+int __attribute__ ((noreturn)) foo2 (void); /* { dg-warning "attribute .noreturn. follows declaration with attribute .warn_unused_result." } */