To make review easier I broke out the C++ changes for the attributes work into a patch of their own. I also found the API I had asked about, to look up a declaration based on one that's about to be added/merged.
This patch depends on the foundation bits posted here: https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00599.html Thanks Martin PS The decls_match changes were required only because the x86_64 target's targetm.target_option.function_versions() fails with a hard error when it finds a conflict in the target attribute while comparing declarations. That doesn't seem quite right for a predicate-type of an API but I didn't see a better way around it than to introduce a new argument and avoid calling into the back end when comparing attributes. PPS I found a surprising number of bugs in the C++ front end while working on this project: 81873, 81762, 81761, and 81609 so I split up the tests I had initially added under c-c++-common between C and C++ and left in c-c++-common only the subset that passes in both languages.
PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted gcc/cp/ChangeLog: PR c/81544 * cp-tree.h (decls_match): Add default argument. * decl.c (decls_match): Avoid calling into the target back end and triggering an error. * decl2.c (cplus_decl_attributes): Look up existing declaration and pass it to decl_attributes. * tree.c (cxx_attribute_table): Initialize new member of struct attribute_spec. gcc/testsuite/ChangeLog: PR c/81544 * g++.dg/Wattributes-2.C: New test. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 115cdaf..f2bac2a 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6080,7 +6080,7 @@ extern void finish_scope (void); extern void push_switch (tree); extern void pop_switch (void); extern tree make_lambda_name (void); -extern int decls_match (tree, tree); +extern int decls_match (tree, tree, bool = true); extern tree duplicate_decls (tree, tree, bool); extern tree declare_local_label (tree); extern tree define_label (location_t, tree); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index aab2019..c9c8cd1 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -993,7 +993,7 @@ push_local_name (tree decl) `const int&'. */ int -decls_match (tree newdecl, tree olddecl) +decls_match (tree newdecl, tree olddecl, bool record_versions /* = true */) { int types_match; @@ -1088,6 +1088,7 @@ decls_match (tree newdecl, tree olddecl) if (types_match && !DECL_EXTERN_C_P (newdecl) && !DECL_EXTERN_C_P (olddecl) + && record_versions && targetm.target_option.function_versions (newdecl, olddecl)) { /* Mark functions as versions if necessary. Modify the mangled decl diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index 2a52f8c..82df5e3 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -1404,7 +1404,31 @@ cplus_decl_attributes (tree *decl, tree attributes, int flags) attributes, flags); } else - decl_attributes (decl, attributes, flags); + { + tree last_decl = (DECL_P (*decl) && DECL_NAME (*decl) + ? lookup_name (DECL_NAME (*decl)) : NULL_TREE); + + if (last_decl && TREE_CODE (last_decl) == OVERLOAD) + for (ovl_iterator iter (last_decl, true); ; ++iter) + { + if (!iter) + { + last_decl = NULL_TREE; + break; + } + + if (TREE_CODE (*iter) == OVERLOAD) + continue; + + if (decls_match (*decl, *iter, /*record_decls=*/false)) + { + last_decl = *iter; + break; + } + } + + decl_attributes (decl, attributes, flags, last_decl); + } if (TREE_CODE (*decl) == TYPE_DECL) SET_IDENTIFIER_TYPE_VALUE (DECL_NAME (*decl), TREE_TYPE (*decl)); diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 8f18665..6cbf4b2 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -4314,10 +4314,10 @@ const struct attribute_spec cxx_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "init_priority", 1, 1, true, false, false, - handle_init_priority_attribute, false }, + handle_init_priority_attribute, false, NULL }, { "abi_tag", 1, -1, false, false, false, - handle_abi_tag_attribute, true }, - { NULL, 0, 0, false, false, false, NULL, false } + handle_abi_tag_attribute, true, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Table of C++ standard attributes. */ @@ -4326,10 +4326,10 @@ const struct attribute_spec std_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "maybe_unused", 0, 0, false, false, false, - handle_unused_attribute, false }, + handle_unused_attribute, false, NULL }, { "nodiscard", 0, 0, false, false, false, - handle_nodiscard_attribute, false }, - { NULL, 0, 0, false, false, false, NULL, false } + handle_nodiscard_attribute, false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Handle an "init_priority" attribute; arguments as in diff --git a/gcc/testsuite/g++.dg/Wattributes-2.C b/gcc/testsuite/g++.dg/Wattributes-2.C new file mode 100644 index 0000000..1470b16 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wattributes-2.C @@ -0,0 +1,35 @@ +// Test to verify that attributes on distinct overloads of a function +// with the same name are properly looked up and applied. +// { dg-do compile } +// { dg-options "-Wall" } + +int +foo (int); + +int __attribute__ ((noreturn)) +foo (int, int); + +int __attribute__ ((warn_unused_result)) +foo (int, int, int); + +int call_foo_1 () +{ + foo (1); +} // { dg-warning "\\\[-Wreturn-type]" } + +int call_foo_2 () +{ + foo (1, 2); +} + +int call_foo_3 () +{ + foo (1, 2, 3); // { dg-warning "\\\[-Wunused-result]" } +} // { dg-warning "\\\[-Wreturn-type]" } + +int call_foo_4 () +{ + // Make sure an error doesn't trigger bogus warnings or an ICE. + foo (1, 2, 3, 4); // { dg-error "no matching function" } + return 0; +}