| Message ID | c5de6b02-9c3d-4e91-95b4-3fa138981af8@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] avoid applying attributes to explicit specializations (PR 83871) | expand |
On 02/04/2018 07:07 PM, Martin Sebor wrote: > To resolve the underlying root cause of the P1 bug c++/83503 > - bogus -Wattributes for const and pure on function template > specialization, that we discussed last week, I've taken a stab > at making the change to avoid applying primary template's > attributes to its explicit specializations. (The bug tracking > the underlying root cause is 83871 - wrong code for attribute > const and pure on distinct template specializations). > > The attached patch is what I have so far. It's incomplete > and not all the tests pass for a couple of reasons: > > 1) it only handles function templates (not class templates), > and I have no tests for those yet, Class templates may already work the way you expect; at least aligned does, though that doesn't involve TYPE_ATTRIBUTES. Hmm, it seems that we currently don't propagate unused even to implicit instantiations, a bug in the other direction: template <class T> struct [[gnu::unused]] A { }; int main() { A<int> a; // shouldn't warn } > 2) it isn't effective for the nonnull and returns_nonnull > attributes because they are treated as type attributes, > 3) the change to deal with attributes on function arguments > may be unnecessary (I couldn't come up with any that would > be propagated from the primary -- are there some?). Yes, I think this is unnecessary. > Before I proceed with it I first want to make sure that it should > be fixed for GCC 8, Some of it, I think. Probably not the whole issue. > that duplicate_decls is the right place for these changes I think so. > and that (2) should be fixed by treating those > and other such attributes by applying them to function decls. This seems out of bounds for GCC 8. It would also mean that we couldn't use such attributes on pointers to functions. > + TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); TREE_NOTHROW is mostly a non-attribute property, so I'd leave it out of this. > + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); If a template is declared to be malloc, IMO we should really warn if a specialization is missing that attribute, it seems certain to be a mistake. In general, I think we should (optionally) warn if a template has attributes and a specialization doesn't, as a user might have been relying on the current behavior. > + if (!merge_attr) > + { > + /* Remove the two function attributes that are, in fact, > + treated as (quasi) type attributes. */ > + tree attrs = TYPE_ATTRIBUTES (newtype); > + tree newattrs = remove_attribute ("nonnull", attrs); > + newattrs = remove_attribute ("returns_nonnull", attrs); > + if (newattrs != attrs) > + TYPE_ATTRIBUTES (newtype) = newattrs; > + } Instead of this, we should avoid calling merge_types and just use TREE_TYPE (newdecl) for newtype. Jason
On Sun, 4 Feb 2018, Martin Sebor wrote: > We've talked about (2) in the past (bug 71463) but this seems > like an opportunity to revisit it and (hopefully) make a change > to treat these the same as all other function attributes rather > than type attributes. Besides fixing the wrong code bugs and I'd say that actually more attributes should be made into type attributes where they are currently function attributes - anything that affects optimizations or warnings in the caller based on properties of the callee is something where it's meaningful to have a pointer-to-function where the pointed-to function type has that property.
On 02/05/2018 04:44 PM, Joseph Myers wrote: > On Sun, 4 Feb 2018, Martin Sebor wrote: > >> We've talked about (2) in the past (bug 71463) but this seems >> like an opportunity to revisit it and (hopefully) make a change >> to treat these the same as all other function attributes rather >> than type attributes. Besides fixing the wrong code bugs and > > I'd say that actually more attributes should be made into type attributes > where they are currently function attributes - anything that affects > optimizations or warnings in the caller based on properties of the callee > is something where it's meaningful to have a pointer-to-function where the > pointed-to function type has that property. What then happens when the pointer to which the attributes are transferred (presumably by initialization) is then assigned the address of a function that has different attributes that? Based on testing, transferring type attributes doesn't seem to have a beneficial effect on the generated code. For example, G++ treats const and pure as function attributes but returns_nonnull as a type attribute. Calls made from function templates via a pointer are treated according to the constness and "pureness" of the actual argument to the template (i.e., that of the function passed to it as an argument). But similar calls seem to disregard the returns_nonnull attribute and lead to suboptimal code when a function with the attribute is passed to the template. I haven't spent enough time to understand why that is yet, but anecdotally what I have seen suggests using type attributes leads to problems. At least in C++. Martin
On Mon, 5 Feb 2018, Martin Sebor wrote: > On 02/05/2018 04:44 PM, Joseph Myers wrote: > > On Sun, 4 Feb 2018, Martin Sebor wrote: > > > > > We've talked about (2) in the past (bug 71463) but this seems > > > like an opportunity to revisit it and (hopefully) make a change > > > to treat these the same as all other function attributes rather > > > than type attributes. Besides fixing the wrong code bugs and > > > > I'd say that actually more attributes should be made into type attributes > > where they are currently function attributes - anything that affects > > optimizations or warnings in the caller based on properties of the callee > > is something where it's meaningful to have a pointer-to-function where the > > pointed-to function type has that property. > > What then happens when the pointer to which the attributes are > transferred (presumably by initialization) is then assigned > the address of a function that has different attributes that? The pointer object gets the attributes by being declared with the appropriate attribute. (Example: the error member of struct cpp_callbacks in cpplib.h.) If a pointer to a function that lacks the required property is stored in the object with pointer-to-attributed-function type, that's a bug - just as it's a bug if a function is declared with an attribute without satisfying the required properties.
On 02/05/2018 05:55 PM, Joseph Myers wrote: > On Mon, 5 Feb 2018, Martin Sebor wrote: > >> On 02/05/2018 04:44 PM, Joseph Myers wrote: >>> On Sun, 4 Feb 2018, Martin Sebor wrote: >>> >>>> We've talked about (2) in the past (bug 71463) but this seems >>>> like an opportunity to revisit it and (hopefully) make a change >>>> to treat these the same as all other function attributes rather >>>> than type attributes. Besides fixing the wrong code bugs and >>> >>> I'd say that actually more attributes should be made into type attributes >>> where they are currently function attributes - anything that affects >>> optimizations or warnings in the caller based on properties of the callee >>> is something where it's meaningful to have a pointer-to-function where the >>> pointed-to function type has that property. >> >> What then happens when the pointer to which the attributes are >> transferred (presumably by initialization) is then assigned >> the address of a function that has different attributes that? > > The pointer object gets the attributes by being declared with the > appropriate attribute. (Example: the error member of struct cpp_callbacks > in cpplib.h.) If a pointer to a function that lacks the required property > is stored in the object with pointer-to-attributed-function type, that's a > bug - just as it's a bug if a function is declared with an attribute > without satisfying the required properties. Oh, I see what you mean. I thought you were suggesting to transparently transfer the attributes to the pointer type on initialization. I agree that making it possible to explictly specify some of these attributes on function pointers (i.e., making them type attributes) would be useful. Provided, of course, that incompatible conversions were diagnosed. Martin
On 02/05/2018 02:52 PM, Jason Merrill wrote: > On 02/04/2018 07:07 PM, Martin Sebor wrote: >> To resolve the underlying root cause of the P1 bug c++/83503 >> - bogus -Wattributes for const and pure on function template >> specialization, that we discussed last week, I've taken a stab >> at making the change to avoid applying primary template's >> attributes to its explicit specializations. (The bug tracking >> the underlying root cause is 83871 - wrong code for attribute >> const and pure on distinct template specializations). >> >> The attached patch is what I have so far. It's incomplete >> and not all the tests pass for a couple of reasons: >> >> 1) it only handles function templates (not class templates), >> and I have no tests for those yet, > > Class templates may already work the way you expect; at least aligned > does, though that doesn't involve TYPE_ATTRIBUTES. > > Hmm, it seems that we currently don't propagate unused even to implicit > instantiations, a bug in the other direction: > > template <class T> struct [[gnu::unused]] A { }; > > int main() > { > A<int> a; // shouldn't warn > } I opened bug 84221 to track it. It's a regression WRT 4.7. For types, it's not completely clear to me what should be expected for attribute deprecated. Not inheriting the attribute means that users would be able to explicitly specialize a deprecated primary template which is in most cases contrary to the intent of the attribute. On the other hand, inheriting it means that there would be no good way to deprecate the primary without also deprecating its explicit specializations (because declaring the explicit specializations would trigger the warning). The use case for this was mentioned by Richard in the core discussion (deprecating the std::numeric_limits primary). I can't think of any way to make it work. The only solution that comes to mind is to use the name of the source file (or header) in which the primary is defined and allow explicit specializations to be defined in it while issuing the warning for those defined in other files. But this definitely seems like GCC 9 material. > >> 2) it isn't effective for the nonnull and returns_nonnull >> attributes because they are treated as type attributes, >> 3) the change to deal with attributes on function arguments >> may be unnecessary (I couldn't come up with any that would >> be propagated from the primary -- are there some?). > > Yes, I think this is unnecessary. Okay, thanks for confirming that. > >> Before I proceed with it I first want to make sure that it should >> be fixed for GCC 8, > > Some of it, I think. Probably not the whole issue. > >> that duplicate_decls is the right place for these changes > > I think so. > >> and that (2) should be fixed by treating those >> and other such attributes by applying them to function decls. > > This seems out of bounds for GCC 8. It would also mean that we couldn't > use such attributes on pointers to functions. > >> + TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); > > TREE_NOTHROW is mostly a non-attribute property, so I'd leave it out of > this. __attribute__ ((nothrow))? The patch includes a test case with wrong-code due to inheriting the attribute. With exception specifications having to match between the primary and its specializations it's the only way to make them different. I've left this unchanged but let me know if I'm missing something. > >> + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); > > If a template is declared to be malloc, IMO we should really warn if a > specialization is missing that attribute, it seems certain to be a mistake. I tend to agree that it's likely a mistake. Though warning in the front-end will lead to false positives if the function isn't malloc. Ideally, this would be detected in the middle- end (where -Wsuggest-attribute=malloc is handled) but I suspect it's too late for that. I've added a simple warning for it. > In general, I think we should (optionally) warn if a template has > attributes and a specialization doesn't, as a user might have been > relying on the current behavior. I've added a new option, -Wmissing-attribute. In bug 81824 Joseph asked for such a warning for C (for function resolvers and aliases) and so I'll use the same option for both (I expect it's too late to handle 81824 in GCC 8 but I'll finish it in GCC 9). Adding the warning required passing some additional attributes around and so more churn. >> + if (!merge_attr) >> + { >> + /* Remove the two function attributes that are, in fact, >> + treated as (quasi) type attributes. */ >> + tree attrs = TYPE_ATTRIBUTES (newtype); >> + tree newattrs = remove_attribute ("nonnull", attrs); >> + newattrs = remove_attribute ("returns_nonnull", attrs); >> + if (newattrs != attrs) >> + TYPE_ATTRIBUTES (newtype) = newattrs; >> + } > > Instead of this, we should avoid calling merge_types and just use > TREE_TYPE (newdecl) for newtype. Ah, great, thanks. That works and fixes the outstanding FAILs in the tests. Attached is an updated patch. It hasn't gone through full testing yet but please let me know if you'd like me to make some changes. Martin PR c++/83871 - wrong code for attribute const and pure on distinct template specializations PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization gcc/ChangeLog: PR c++/83871 * gcc/doc/invoke.texi (-Wmissing-attribute): New option. gcc/c-family/ChangeLog: PR c++/83871 * c.opt (-Wmissing-attribute): New option. gcc/cp/ChangeLog: PR c++/83871 PR c++/83503 * cp-tree.h (duplicate_decls): Add an argument. (check_explicit_specialization): Same. * decl.c (warn_spec_missing_attributes): New function. (duplicate_decls): Avoid applying primary function template's attributes to its explicit specializations. (register_specialization): Add argument. gcc/testsuite/ChangeLog: PR c++/83871 PR c++/83503 * g++.dg/ext/attr-const-pure.C: New test. * g++.dg/ext/attr-malloc.C: New test. * g++.dg/ext/attr-nonnull.C: New test. * g++.dg/ext/attr-nothrow.C: New test. * g++.dg/ext/attr-returns-nonnull.C: New test. * g++.dg/Wmissing-attribute.C: New test. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9c71726..a4d5e61 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -781,6 +781,11 @@ Wtemplates C++ ObjC++ Var(warn_templates) Warning Warn on primary template declaration. +Wmissing-attribute +C ObjC C++ ObjC++ Var(warn_missing_attribute) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn about declarations of entities that may be missing attributes +that related entities have been declared with it. + Wmissing-format-attribute C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format) ; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a53f4fd..fe75a9b 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6137,7 +6137,8 @@ extern void note_iteration_stmt_body_end (bool); extern tree make_lambda_name (void); extern int decls_match (tree, tree, bool = true); extern bool maybe_version_functions (tree, tree); -extern tree duplicate_decls (tree, tree, bool); +extern tree duplicate_decls (tree, tree, bool, + tree = NULL_TREE); extern tree declare_local_label (tree); extern tree define_label (location_t, tree); extern void check_goto (tree); @@ -6470,7 +6471,8 @@ extern void end_specialization (void); extern void begin_explicit_instantiation (void); extern void end_explicit_instantiation (void); extern void check_unqualified_spec_or_inst (tree, location_t); -extern tree check_explicit_specialization (tree, tree, int, int); +extern tree check_explicit_specialization (tree, tree, int, int, + tree = NULL_TREE); extern int num_template_headers_for_class (tree); extern void check_template_variable (tree); extern tree make_auto (void); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 244a3ef..f6461b6 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -51,6 +51,7 @@ along with GCC; see the file COPYING3. If not see #include "builtins.h" #include "gimplify.h" #include "asan.h" +#include <string> /* Possible cases of bad specifiers type used by bad_specifiers. */ enum bad_spec_place { @@ -1357,6 +1358,104 @@ check_redeclaration_no_default_args (tree decl) } } +/* Warn for a template specialization SPEC that is missing some of a set + of function or type attributes that the template TEMPL is declared with. + ATTRLIST is a list of additional attributes that SPEC should be taken + to ultimately be declared with. */ + +static void +warn_spec_missing_attributes (tree tmpl, tree spec, tree attrlist) +{ + /* Avoid warning if either declaration or its type is deprecated. */ + if (TREE_DEPRECATED (tmpl) + || TREE_DEPRECATED (spec)) + return; + + tree tmpl_type = TREE_TYPE (tmpl); + tree spec_type = TREE_TYPE (spec); + + if (TREE_DEPRECATED (tmpl_type) + || TREE_DEPRECATED (spec_type) + || TREE_DEPRECATED (TREE_TYPE (tmpl_type)) + || TREE_DEPRECATED (TREE_TYPE (spec_type))) + return; + + tree tmpl_attrs[] = { DECL_ATTRIBUTES (tmpl), TYPE_ATTRIBUTES (tmpl_type) }; + tree spec_attrs[] = { DECL_ATTRIBUTES (spec), TYPE_ATTRIBUTES (spec_type) }; + + if (!spec_attrs[0]) + spec_attrs[0] = attrlist; + else if (!spec_attrs[1]) + spec_attrs[1] = attrlist; + + /* Avoid warning if the primary has no attributes. */ + if (!tmpl_attrs[0] && !tmpl_attrs[1]) + return; + + /* Avoid warning if either declaration contains an attribute on + the white list below. */ + const char* const whitelist[] = { + "error", "noreturn", "warning" + }; + + for (unsigned i = 0; i != 2; ++i) + for (unsigned j = 0; j != sizeof whitelist / sizeof *whitelist; ++j) + if (lookup_attribute (whitelist[j], tmpl_attrs[i]) + || lookup_attribute (whitelist[j], spec_attrs[i])) + return; + + /* Avoid warning if the difference between the primary and + the specialization is not in one of the attributes below. */ + const char* const blacklist[] = { + "alloc_align", "alloc_size", "assume_aligned", "format", + "format_arg", "malloc", "nonnull", "warn_unused_result" + }; + + /* Put together a list of the black listed attributes that the primary + template is declared with that the specialization is not, in case + it's not apparent from the most recent declaration of the primary. */ + unsigned nattrs = 0; + std::string str; + + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) + { + for (unsigned j = 0; j != 2; ++j) + { + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) + continue; + + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) + { + if (lookup_attribute (blacklist[i], spec_attrs[k])) + break; + + if (str.size ()) + str += ", "; + str += "%<"; + str += blacklist[i]; + str += "%>"; + ++nattrs; + } + } + } + + if (!nattrs) + return; + + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attribute, + "explicit specialization %q#D may be missing attributes", + spec)) + { + if (nattrs > 1) + str = G_("missing primary template attributes ") + str; + else + str = G_("missing primary template attribute ") + str; + + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); + } + +} + #define GNU_INLINE_P(fn) (DECL_DECLARED_INLINE_P (fn) \ && lookup_attribute ("gnu_inline", \ DECL_ATTRIBUTES (fn))) @@ -1368,10 +1467,14 @@ check_redeclaration_no_default_args (tree decl) If NEWDECL is not a redeclaration of OLDDECL, NULL_TREE is returned. - NEWDECL_IS_FRIEND is true if NEWDECL was declared as a friend. */ + NEWDECL_IS_FRIEND is true if NEWDECL was declared as a friend. + + ATTRLIST is an optional list of attributes that NEWDECL is being + declared with. */ tree -duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) +duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend, + tree attrlist /* = NULL_TREE */) { unsigned olddecl_uid = DECL_UID (olddecl); int olddecl_friend = 0, types_match = 0, hidden_friend = 0; @@ -1971,10 +2074,25 @@ next_arg:; DECL_ORIGINAL_TYPE (newdecl) = DECL_ORIGINAL_TYPE (olddecl); } - /* Copy all the DECL_... slots specified in the new decl - except for any that we copy here from the old type. */ - DECL_ATTRIBUTES (newdecl) - = (*targetm.merge_decl_attributes) (olddecl, newdecl); + /* True to merge attributes between the declarations, false to + set OLDDECL's attributes to those of NEWDECL (for template + explicit specializations that specify their own attributes + independent of those specified for the primary template). */ + const bool merge_attr = (TREE_CODE (newdecl) != FUNCTION_DECL + || !DECL_TEMPLATE_SPECIALIZATION (newdecl) + || DECL_TEMPLATE_SPECIALIZATION (olddecl)); + + /* Copy all the DECL_... slots specified in the new decl except for + any that we copy here from the old type. */ + if (merge_attr) + DECL_ATTRIBUTES (newdecl) + = (*targetm.merge_decl_attributes) (olddecl, newdecl); + else + { + warn_spec_missing_attributes (olddecl, newdecl, attrlist); + + DECL_ATTRIBUTES (olddecl) = DECL_ATTRIBUTES (newdecl); + } if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl)) { @@ -2103,7 +2221,7 @@ next_arg:; } else /* Merge the data types specified in the two decls. */ - newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); + newtype = TREE_TYPE (newdecl); if (VAR_P (newdecl)) { @@ -2167,13 +2285,24 @@ next_arg:; && !(processing_template_decl && uses_template_parms (newdecl))) layout_decl (newdecl, 0); - /* Merge the type qualifiers. */ - if (TREE_READONLY (newdecl)) - TREE_READONLY (olddecl) = 1; if (TREE_THIS_VOLATILE (newdecl)) TREE_THIS_VOLATILE (olddecl) = 1; - if (TREE_NOTHROW (newdecl)) - TREE_NOTHROW (olddecl) = 1; + + if (merge_attr) + { + /* Merge the type qualifiers. */ + if (TREE_READONLY (newdecl)) + TREE_READONLY (olddecl) = 1; + if (TREE_NOTHROW (newdecl)) + TREE_NOTHROW (olddecl) = 1; + } + else + { + /* Set the bits that correspond to the const and nothrow + function attributes. */ + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + } /* Merge deprecatedness. */ if (TREE_DEPRECATED (newdecl)) @@ -2212,13 +2341,22 @@ next_arg:; |= DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (olddecl); DECL_NO_LIMIT_STACK (newdecl) |= DECL_NO_LIMIT_STACK (olddecl); TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); - TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); - DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); - DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); DECL_LOOPING_CONST_OR_PURE_P (newdecl) |= DECL_LOOPING_CONST_OR_PURE_P (olddecl); + if (merge_attr) + { + TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); + DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); + DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); + } + else + { + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); + DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl); + } /* Keep the old RTL. */ COPY_DECL_RTL (olddecl, newdecl); } @@ -2340,17 +2478,18 @@ next_arg:; { tree parm; - /* Merge parameter attributes. */ + /* Merge or assign parameter attributes. */ tree oldarg, newarg; - for (oldarg = DECL_ARGUMENTS(olddecl), - newarg = DECL_ARGUMENTS(newdecl); - oldarg && newarg; - oldarg = DECL_CHAIN(oldarg), newarg = DECL_CHAIN(newarg)) { - DECL_ATTRIBUTES (newarg) - = (*targetm.merge_decl_attributes) (oldarg, newarg); - DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); - } - + for (oldarg = DECL_ARGUMENTS (olddecl), + newarg = DECL_ARGUMENTS (newdecl); + oldarg && newarg; + oldarg = DECL_CHAIN (oldarg), newarg = DECL_CHAIN (newarg)) + { + DECL_ATTRIBUTES (newarg) + = (*targetm.merge_decl_attributes) (oldarg, newarg); + DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); + } + if (DECL_TEMPLATE_INSTANTIATION (olddecl) && !DECL_TEMPLATE_INSTANTIATION (newdecl)) { @@ -8912,7 +9051,8 @@ grokfndecl (tree ctype, template_count, 2 * funcdef_flag + 4 * (friendp != 0) + - 8 * concept_p); + 8 * concept_p, + *attrlist); if (decl == error_mark_node) return NULL_TREE; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index ca73bb1..8b3cb54 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -1473,15 +1473,17 @@ is_specialization_of_friend (tree decl, tree friend_decl) /* Register the specialization SPEC as a specialization of TMPL with the indicated ARGS. IS_FRIEND indicates whether the specialization - is actually just a friend declaration. Returns SPEC, or an - equivalent prior declaration, if available. + is actually just a friend declaration. ATTRLIST is the list of + attributes that the specialization is declared with or NULL when + it isn't. Returns SPEC, or an equivalent prior declaration, if + available. We also store instantiations of field packs in the hash table, even though they are not themselves templates, to make lookup easier. */ static tree register_specialization (tree spec, tree tmpl, tree args, bool is_friend, - hashval_t hash) + hashval_t hash, tree attrlist = NULL_TREE) { tree fn; spec_entry **slot = NULL; @@ -1572,7 +1574,7 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, for the specialization, we want this to look as if there were no definition, and vice versa. */ DECL_INITIAL (fn) = NULL_TREE; - duplicate_decls (spec, fn, is_friend); + duplicate_decls (spec, fn, is_friend, attrlist); /* The call to duplicate_decls will have applied [temp.expl.spec]: @@ -2651,7 +2653,8 @@ tree check_explicit_specialization (tree declarator, tree decl, int template_count, - int flags) + int flags, + tree attrlist) { int have_def = flags & 2; int is_friend = flags & 4; @@ -3109,7 +3112,7 @@ check_explicit_specialization (tree declarator, process_partial_specialization. */ if (!processing_template_decl) decl = register_specialization (decl, gen_tmpl, targs, - is_friend, 0); + is_friend, 0, attrlist); /* A 'structor should already have clones. */ gcc_assert (decl == error_mark_node diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f3d9336..f50e3f3 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -295,7 +295,7 @@ Objective-C and Objective-C++ Dialects}. -Winvalid-pch -Wlarger-than=@var{len} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol --Wmisleading-indentation -Wmissing-braces @gol +-Wmisleading-indentation -Wmissing-attribute -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-include-dirs @gol -Wno-multichar -Wmultistatement-macros -Wnonnull -Wnonnull-compare @gol -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol @@ -3887,6 +3887,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. -Wmemset-elt-size @gol -Wmemset-transposed-args @gol -Wmisleading-indentation @r{(only for C/C++)} @gol +-Wmissing-attribute @gol -Wmissing-braces @r{(only for C/ObjC)} @gol -Wmultistatement-macros @gol -Wnarrowing @r{(only for C++)} @gol @@ -4550,6 +4551,17 @@ about the layout of the file that the directive references. This warning is enabled by @option{-Wall} in C and C++. +@item -Wmissing-attribute +@opindex Wmissing-attribute +@opindex Wno-missing-attribute +Warn when a declaration of an entity is missing attributes that a related +entity is declared with whose absence may adversely affect the correctness +of generated code. For example, in C++, the warning is issued when +an explicit specialization of a primary template declared with attribute +@code{malloc} is declared without the attribute. + +This warning is enabled by @option{-Wall}. + @item -Wmissing-braces @opindex Wmissing-braces @opindex Wno-missing-braces diff --git a/gcc/testsuite/g++.dg/Wmissing-attribute.C b/gcc/testsuite/g++.dg/Wmissing-attribute.C new file mode 100644 index 0000000..0915a90 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wmissing-attribute.C @@ -0,0 +1,121 @@ +// PR c++/83871 - wrong code for attribute const and pure on distinct +// template specializations +// Test to verify that a declaration of an explicit specialization with +// no attributes is diagnosed when the primary template is declared with +// one or more attributes. The warning helps highlight a change in GCC +// 8 from previous versions that copied the attributes from the primary +// to the specialization. It also helps point out simply forgetting to +// declare the specialization with an attribute. +// { dg-do compile } +// { dg-options "-Wmissing-attribute" } + +#define ATTR(list) __attribute__ (list) + + +// Verify that a primary without attributes doesn't cause warnings. +template <class T> void fnoattr (); + +template <> void fnoattr<void>(); +template <> void ATTR ((cold)) fnoattr<int>(); +template <> void ATTR ((hot)) fnoattr<double>(); + +// Verify that a noreturn primary also doesn't cause warnings. +template <class T> int ATTR ((noreturn)) fnoreturn (); + +template <> int fnoreturn<void>(); +template <> int ATTR ((cold)) fnoreturn<int>(); +template <> int ATTR ((hot)) fnoreturn<double>(); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_all (int, int); // { dg-message "missing primary template attributes \(.malloc., .alloc_size.|.alloc_size., .malloc.\)" } + +template <> +void* +missing_all<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +// Verify that specifying the same attributes in whatever order +// doesn't trigger the warning +template <> +void* +ATTR ((alloc_size (1), malloc)) +missing_all<char>(int, int); + +template <> +void* +ATTR ((alloc_size (1))) ATTR ((malloc)) ATTR ((returns_nonnull)) +missing_all<signed char>(int, int); + +template <> +void* +ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc)) +missing_all<signed char>(int, int); + +// Verify that the following attributes suppress the warning. +template <> void* ATTR ((error (""))) missing_all<short>(int, int); +template <> void* ATTR ((deprecated)) missing_all<int>(int, int); +template <> void* ATTR ((noreturn)) missing_all<long>(int, int); +template <> void* ATTR ((warning (""))) missing_all<double>(int, int); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_malloc (int, int); // { dg-message "missing primary template attribute .malloc." } + +template <> +void* +ATTR ((alloc_size (1))) +missing_malloc<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((malloc, alloc_size (1))) missing_malloc<int>(int, int); +template <> void* ATTR ((deprecated)) missing_malloc<char>(int, int); +template <> void* ATTR ((error (""))) missing_malloc<short>(int, int); +template <> void* ATTR ((noreturn)) missing_malloc<long>(int, int); +template <> void* ATTR ((warning (""))) missing_malloc<float>(int, int); + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_alloc_size (int, int); // { dg-message "missing primary template attribute .alloc_size." } + +template <> +void* +ATTR ((malloc)) +missing_alloc_size<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + + +template <class T> +void* +ATTR ((nonnull (1))) +missing_nonnull (int*, int*); // { dg-message "missing primary template attribute .nonnull." } + +template <> +void* +ATTR ((malloc)) +missing_nonnull<char>(int*, int*); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((nonnull (1))) missing_nonnull<short>(int*, int*); +template <> void* ATTR ((deprecated)) missing_nonnull<int>(int*, int*); +template <> void* ATTR ((error (""))) missing_nonnull<long>(int*, int*); +template <> void* ATTR ((noreturn)) missing_nonnull<float>(int*, int*); +template <> void* ATTR ((warning (""))) missing_nonnull<void>(int*, int*); + + +template <class T> +void* +ATTR ((warn_unused_result)) +missing_wur (int*, int*); // { dg-message "missing primary template attribute .warn_unused_result." } + +template <> +void* +ATTR ((returns_nonnull)) +missing_wur<char>(int*, int*); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((warn_unused_result)) missing_nonnull<int>(int*, int*); +template <> void* ATTR ((deprecated)) missing_nonnull<long>(int*, int*); +template <> void* ATTR ((error (""))) missing_nonnull<float>(int*, int*); +template <> void* ATTR ((noreturn)) missing_nonnull<double>(int*, int*); +template <> void* ATTR ((warning (""))) missing_nonnull<void>(int*, int*); diff --git a/gcc/testsuite/g++.dg/ext/attr-const-pure.C b/gcc/testsuite/g++.dg/ext/attr-const-pure.C new file mode 100644 index 0000000..6afef6d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const-pure.C @@ -0,0 +1,103 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +int __attribute__ ((pure)) +f (T); + +template <> +int __attribute__ ((const)) f<int> (int); // { dg-bogus "ignoring attribute .const." } + + +template <class T> +int __attribute__ ((const)) +g (T); + +template <> +int __attribute__ ((pure)) g<int> (int); // { dg-bogus "ignoring attribute .const." } + +template <class T> +int __attribute__ ((const)) +h (T); + +template <class T> +int __attribute__ ((pure)) +h (const T*); + +template <> +int h<int> (int); + +template <> +int h<int*> (int*); + +int global; + +extern void h_primary_elim (); +extern void h_cstptr_elim (); +extern void h_cstptr_keep (); +extern void h_int_keep (); +extern void h_intptr_keep (); + +void call_primary_elim (double x) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. + int i0 = h (x); + int i1 = h (x); + + if (i0 != i1) // must be folded into false + h_primary_elim (); // must be eliminated +} + +void call_cstptr_elim (const void *p) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. This verifies that h<const + // void*>*() is treated as const in this context. + int i0 = h (p); + int i1 = h (p); + + if (i0 != i1) // must be folded into false + h_cstptr_elim (); // must be eliminated +} + +void call_cstptr_keep (const void *p) +{ + // Because of the store to the global, both calls to h(p) must + // be emitted. This verifies that h<const void*>*() is not + // treated as const. + int i0 = h (p); + global = 123; + int i1 = h (p); + + if (i0 != i1) // must not be folded + h_cstptr_keep (); // must be emitted +} + +void call_int_keep (int i) +{ + // Both calls to h(i) must be emitted. + int i0 = h (i); + int i1 = h (i); + + if (i0 != i1) // must not be folded + h_int_keep (); // must be emitted +} + +void call_intptr_keep (int *ip) +{ + // Both calls to h(ip) must be emitted. + int i0 = h (ip); + int i1 = h (ip); + + if (i0 != i1) // must not be folded + h_intptr_keep (); // must be emitted +} + +// { dg-final { scan-tree-dump-not "h_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "h_cstptr_elim" "optimized" } } +// { dg-final { scan-tree-dump-times "h_cstptr_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_int_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_intptr_keep" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc.C b/gcc/testsuite/g++.dg/ext/attr-malloc.C new file mode 100644 index 0000000..ce76abe --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc.C @@ -0,0 +1,45 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that an explicit template specifialization does not +// "inherit" attribute malloc from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((malloc)) +f (unsigned); + +template <> +void* +f<int>(unsigned); // { dg-warning "may be missing attributes" } + +static char a[8]; + +void f_void_malloc (); +void f_int_not_malloc (); + +void fv (void) +{ + void *p = f<void>(1); + if (!p) + return; + + if (p == a) // must be false + f_void_malloc (); // should be eliminated +} + + +void fi (void) +{ + void *p = f<int>(1); + if (!p) + return; + + if (p == a) // can be true + f_int_not_malloc (); // must not be eliminated +} + +// Verify that the call to f_void_not_malloc() is eliminated but +// the call to f_int_not_malloc() is retained. +// { dg-final { scan-tree-dump-not "f_void_malloc" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_not_malloc" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-nonnull.C new file mode 100644 index 0000000..57d2cb0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nonnull.C @@ -0,0 +1,31 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nonnull (1))) +f (T*, T*, T*); + +template <> +void +f<int>(int*, int*, int*); // { dg-warning "may be missing attributes" } + +template <> +void __attribute__ ((nonnull (3))) +f<float>(float*, float*, float*); + + +void test_nonnull (void) +{ + f<void>(0, 0, 0); // { dg-warning "null argument where non-null required \\\(argument 1\\\)" } + + f<int>(0, 0, 0); // { dg-bogus "null argument" } + + f<float>(0, 0, 0); + // { dg-bogus "null argument where non-null required \\\(argument 1\\\)" "" { target *-*-* } .-1 } + // { dg-warning "null argument where non-null required \\\(argument 3\\\)" "" { target *-*-* } .-2 } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow.C b/gcc/testsuite/g++.dg/ext/attr-nothrow.C new file mode 100644 index 0000000..ae6b674 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow.C @@ -0,0 +1,47 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute nothrow from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nothrow)) +f (); + +template <> +void +f<int>(); + +void f_void_nothrow (); +void f_int_maythrow (); + +void fv (void) +{ + try + { + f<void>(); + } + catch (...) // cannot be be reached + { + f_void_nothrow (); // should be eliminated + } +} + + +void fi (void) +{ + try + { + f<int>(); + } + catch (...) // may be reached + { + f_int_maythrow (); // must not be eliminated + } +} + +// Verify that the call to f_void_nothrow() is eliminated but +// the call to f_int_maythrow() is retained. +// { dg-final { scan-tree-dump-not "f_void_nothrow" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_maythrow" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C new file mode 100644 index 0000000..f75f32e --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C @@ -0,0 +1,42 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((returns_nonnull)) +g (); + +template <> +void* +g<int>(); + +extern void g_void_returns_nonnull (); +extern void g_int_may_return_null (); + +void test_returns_nonnull () +{ + void *p = g<void>(); + if (!p) + g_void_returns_nonnull (); + + (void)&p; +} + +void test_may_return_null () +{ + void *p = g<int>(); + if (!p) + g_int_may_return_null (); + + (void)&p; +} + + +// Verify that the call to g_void_returns_nonnull() is eliminated but +// the call to g_int_may_return_null() is retained. +// { dg-final { scan-tree-dump-not "g_void_returns_nonnull" "optimized" } } +// { dg-final { scan-tree-dump-times "g_int_may_return_null" 1 "optimized" } }
On 02/06/2018 11:01 PM, Martin Sebor wrote: > On 02/05/2018 02:52 PM, Jason Merrill wrote: >> On 02/04/2018 07:07 PM, Martin Sebor wrote: >>> To resolve the underlying root cause of the P1 bug c++/83503 >>> - bogus -Wattributes for const and pure on function template >>> specialization, that we discussed last week, I've taken a stab >>> at making the change to avoid applying primary template's >>> attributes to its explicit specializations. (The bug tracking >>> the underlying root cause is 83871 - wrong code for attribute >>> const and pure on distinct template specializations). >>> >>> The attached patch is what I have so far. It's incomplete >>> and not all the tests pass for a couple of reasons: >>> >>> 1) it only handles function templates (not class templates), >>> and I have no tests for those yet, >> >> Class templates may already work the way you expect; at least aligned >> does, though that doesn't involve TYPE_ATTRIBUTES. >> >> Hmm, it seems that we currently don't propagate unused even to implicit >> instantiations, a bug in the other direction: >> >> template <class T> struct [[gnu::unused]] A { }; >> >> int main() >> { >> A<int> a; // shouldn't warn >> } > > I opened bug 84221 to track it. It's a regression WRT 4.7. > > For types, it's not completely clear to me what should be > expected for attribute deprecated. Not inheriting the > attribute means that users would be able to explicitly > specialize a deprecated primary template which is in most > cases contrary to the intent of the attribute. > > On the other hand, inheriting it means that there would be > no good way to deprecate the primary without also deprecating > its explicit specializations (because declaring the explicit > specializations would trigger the warning). The use case > for this was mentioned by Richard in the core discussion > (deprecating the std::numeric_limits primary). > > I can't think of any way to make it work. The only solution > that comes to mind is to use the name of the source file (or > header) in which the primary is defined and allow explicit > specializations to be defined in it while issuing the warning > for those defined in other files. But this definitely seems > like GCC 9 material. Yes, we definitely don't need to mess with this now. >>> 2) it isn't effective for the nonnull and returns_nonnull >>> attributes because they are treated as type attributes, >>> 3) the change to deal with attributes on function arguments >>> may be unnecessary (I couldn't come up with any that would >>> be propagated from the primary -- are there some?). >> >> Yes, I think this is unnecessary. > > Okay, thanks for confirming that. > >>> Before I proceed with it I first want to make sure that it should >>> be fixed for GCC 8, >> >> Some of it, I think. Probably not the whole issue. >> >>> that duplicate_decls is the right place for these changes >> >> I think so. >> >>> and that (2) should be fixed by treating those >>> and other such attributes by applying them to function decls. >> >> This seems out of bounds for GCC 8. It would also mean that we couldn't >> use such attributes on pointers to functions. >> >>> + TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); >> >> TREE_NOTHROW is mostly a non-attribute property, so I'd leave it out of >> this. > > __attribute__ ((nothrow))? The patch includes a test case with > wrong-code due to inheriting the attribute. With exception > specifications having to match between the primary and its > specializations it's the only way to make them different. > I've left this unchanged but let me know if I'm missing > something. Yeah, I think you're right. But I notice that the existing code (and thus your patch) touches TREE_NOTHROW in two places, and the first one seems wrong; we only want it inside the FUNCTION_DECL section. >>> + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); >> >> If a template is declared to be malloc, IMO we should really warn if a >> specialization is missing that attribute, it seems certain to be a >> mistake. > > I tend to agree that it's likely a mistake. Though warning > in the front-end will lead to false positives if the function > isn't malloc. Ideally, this would be detected in the middle- > end (where -Wsuggest-attribute=malloc is handled) but I suspect > it's too late for that. I've added a simple warning for it. > >> In general, I think we should (optionally) warn if a template has >> attributes and a specialization doesn't, as a user might have been >> relying on the current behavior. > > I've added a new option, -Wmissing-attribute. In bug 81824 > Joseph asked for such a warning for C (for function resolvers > and aliases) and so I'll use the same option for both (I expect > it's too late to handle 81824 in GCC 8 but I'll finish it in > GCC 9). Adding the warning required passing some additional > attributes around and so more churn. Rather than pass them down into register_specialization and duplicate_decls, check_explicit_specialization could compare the attribute list to the attributes on the template itself. >>> + if (!merge_attr) >>> + { >>> + /* Remove the two function attributes that are, in fact, >>> + treated as (quasi) type attributes. */ >>> + tree attrs = TYPE_ATTRIBUTES (newtype); >>> + tree newattrs = remove_attribute ("nonnull", attrs); >>> + newattrs = remove_attribute ("returns_nonnull", attrs); >>> + if (newattrs != attrs) >>> + TYPE_ATTRIBUTES (newtype) = newattrs; >>> + } >> >> Instead of this, we should avoid calling merge_types and just use >> TREE_TYPE (newdecl) for newtype. > > Ah, great, thanks. That works and fixes the outstanding FAILs > in the tests. > /* Merge the data types specified in the two decls. */ > - newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); > + newtype = TREE_TYPE (newdecl); I meant to avoid merging only when !merge_attr; we should still merge if merge_attr is true. > Attached is an updated patch. It hasn't gone through full > testing yet but please let me know if you'd like me to make > some changes. > + const char* const whitelist[] = { > + "error", "noreturn", "warning" > + }; Why whitelist noreturn? I would expect to want that to be consistent. Jason
>> __attribute__ ((nothrow))? The patch includes a test case with >> wrong-code due to inheriting the attribute. With exception >> specifications having to match between the primary and its >> specializations it's the only way to make them different. >> I've left this unchanged but let me know if I'm missing >> something. > > Yeah, I think you're right. But I notice that the existing code (and > thus your patch) touches TREE_NOTHROW in two places, and the first one > seems wrong; we only want it inside the FUNCTION_DECL section. I think I see what you mean. I don't follow all the details of all the code here but hopefully I got it right. > Rather than pass them down into register_specialization and > duplicate_decls, check_explicit_specialization could compare the > attribute list to the attributes on the template itself. I took me a while to find DECL_TEMPLATE_RESULT. Hopefully that's the right way to get the primary from a TEMPLATE_DECL. >>>> + if (!merge_attr) >>>> + { >>>> + /* Remove the two function attributes that are, in fact, >>>> + treated as (quasi) type attributes. */ >>>> + tree attrs = TYPE_ATTRIBUTES (newtype); >>>> + tree newattrs = remove_attribute ("nonnull", attrs); >>>> + newattrs = remove_attribute ("returns_nonnull", attrs); >>>> + if (newattrs != attrs) >>>> + TYPE_ATTRIBUTES (newtype) = newattrs; >>>> + } >>> >>> Instead of this, we should avoid calling merge_types and just use >>> TREE_TYPE (newdecl) for newtype. >> >> Ah, great, thanks. That works and fixes the outstanding FAILs >> in the tests. > >> /* Merge the data types specified in the two decls. */ >> - newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); >> + newtype = TREE_TYPE (newdecl); > > I meant to avoid merging only when !merge_attr; we should still merge if > merge_attr is true. Doh! Of course. Silly mistake. Sorry. >> Attached is an updated patch. It hasn't gone through full >> testing yet but please let me know if you'd like me to make >> some changes. > >> + const char* const whitelist[] = { >> + "error", "noreturn", "warning" >> + }; > > Why whitelist noreturn? I would expect to want that to be consistent. I expect noreturn to be used on a primary whose definition is provided but that's not meant to be used the way the API is otherwise expected to be. As in: template <class T> T [[noreturn]] foo () { throw "not implemented"; } template <> int foo<int>(); // implemented elsewhere Beyond that, noreturn can only be paired with a small number of the attributes on the black list (just format and nonnull), otherwise it or the other one is ignored (and -Wattributes is issued). I suppose noreturn would make sense together with format on a template that formatted a message before throwing or printing it and exiting. But format is almost never used in templates so it seems like a stretch. If you think it's important or if you have a use case in mind that I'm not thinking of let me know. Attached is the updated patch, this time bootstrapped and regtested on x86_64-linux. Martin PR c++/83871 - wrong code for attribute const and pure on distinct template specializations PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization gcc/ChangeLog: PR c++/83871 * gcc/doc/invoke.texi (-Wmissing-attribute): New option. gcc/c-family/ChangeLog: PR c++/83871 * c.opt (-Wmissing-attribute): New option. gcc/cp/ChangeLog: PR c++/83871 PR c++/83503 * cp-tree.h (warn_spec_missing_attributes): New function. ((check_explicit_specialization): Add an argument. Call the above function. * decl.c (duplicate_decls): Avoid applying primary function template's attributes to its explicit specializations. gcc/testsuite/ChangeLog: PR c++/83871 PR c++/83503 * g++.dg/ext/attr-const-pure.C: New test. * g++.dg/ext/attr-malloc.C: New test. * g++.dg/ext/attr-nonnull.C: New test. * g++.dg/ext/attr-nothrow.C: New test. * g++.dg/ext/attr-nothrow-2.C: New test. * g++.dg/ext/attr-returns-nonnull.C: New test. * g++.dg/Wmissing-attribute.C: New test. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9c71726..a4d5e61 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -781,6 +781,11 @@ Wtemplates C++ ObjC++ Var(warn_templates) Warning Warn on primary template declaration. +Wmissing-attribute +C ObjC C++ ObjC++ Var(warn_missing_attribute) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn about declarations of entities that may be missing attributes +that related entities have been declared with it. + Wmissing-format-attribute C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format) ; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a53f4fd..87b7916 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6470,7 +6470,8 @@ extern void end_specialization (void); extern void begin_explicit_instantiation (void); extern void end_explicit_instantiation (void); extern void check_unqualified_spec_or_inst (tree, location_t); -extern tree check_explicit_specialization (tree, tree, int, int); +extern tree check_explicit_specialization (tree, tree, int, int, + tree = NULL_TREE); extern int num_template_headers_for_class (tree); extern void check_template_variable (tree); extern tree make_auto (void); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 244a3ef..6e02ba3 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -1971,10 +1971,21 @@ next_arg:; DECL_ORIGINAL_TYPE (newdecl) = DECL_ORIGINAL_TYPE (olddecl); } - /* Copy all the DECL_... slots specified in the new decl - except for any that we copy here from the old type. */ - DECL_ATTRIBUTES (newdecl) - = (*targetm.merge_decl_attributes) (olddecl, newdecl); + /* True to merge attributes between the declarations, false to + set OLDDECL's attributes to those of NEWDECL (for template + explicit specializations that specify their own attributes + independent of those specified for the primary template). */ + const bool merge_attr = (TREE_CODE (newdecl) != FUNCTION_DECL + || !DECL_TEMPLATE_SPECIALIZATION (newdecl) + || DECL_TEMPLATE_SPECIALIZATION (olddecl)); + + /* Copy all the DECL_... slots specified in the new decl except for + any that we copy here from the old type. */ + if (merge_attr) + DECL_ATTRIBUTES (newdecl) + = (*targetm.merge_decl_attributes) (olddecl, newdecl); + else + DECL_ATTRIBUTES (olddecl) = DECL_ATTRIBUTES (newdecl); if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl)) { @@ -2101,9 +2112,10 @@ next_arg:; } } } - else - /* Merge the data types specified in the two decls. */ + else if (merge_attr) newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); + else + newtype = TREE_TYPE (newdecl); if (VAR_P (newdecl)) { @@ -2167,13 +2179,20 @@ next_arg:; && !(processing_template_decl && uses_template_parms (newdecl))) layout_decl (newdecl, 0); - /* Merge the type qualifiers. */ - if (TREE_READONLY (newdecl)) - TREE_READONLY (olddecl) = 1; if (TREE_THIS_VOLATILE (newdecl)) TREE_THIS_VOLATILE (olddecl) = 1; - if (TREE_NOTHROW (newdecl)) - TREE_NOTHROW (olddecl) = 1; + + if (merge_attr) + { + /* Merge the type qualifiers. */ + if (TREE_READONLY (newdecl)) + TREE_READONLY (olddecl) = 1; + } + else + { + /* Set the bits that correspond to the const function attributes. */ + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); + } /* Merge deprecatedness. */ if (TREE_DEPRECATED (newdecl)) @@ -2212,13 +2231,23 @@ next_arg:; |= DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (olddecl); DECL_NO_LIMIT_STACK (newdecl) |= DECL_NO_LIMIT_STACK (olddecl); TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); - TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); - DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); - DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); - DECL_LOOPING_CONST_OR_PURE_P (newdecl) + DECL_LOOPING_CONST_OR_PURE_P (newdecl) |= DECL_LOOPING_CONST_OR_PURE_P (olddecl); + + if (merge_attr) + { + TREE_NOTHROW (olddecl) |= TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); + DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); + } + else + { + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); + DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl); + } /* Keep the old RTL. */ COPY_DECL_RTL (olddecl, newdecl); } @@ -2340,17 +2369,18 @@ next_arg:; { tree parm; - /* Merge parameter attributes. */ + /* Merge or assign parameter attributes. */ tree oldarg, newarg; - for (oldarg = DECL_ARGUMENTS(olddecl), - newarg = DECL_ARGUMENTS(newdecl); - oldarg && newarg; - oldarg = DECL_CHAIN(oldarg), newarg = DECL_CHAIN(newarg)) { - DECL_ATTRIBUTES (newarg) - = (*targetm.merge_decl_attributes) (oldarg, newarg); - DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); - } - + for (oldarg = DECL_ARGUMENTS (olddecl), + newarg = DECL_ARGUMENTS (newdecl); + oldarg && newarg; + oldarg = DECL_CHAIN (oldarg), newarg = DECL_CHAIN (newarg)) + { + DECL_ATTRIBUTES (newarg) + = (*targetm.merge_decl_attributes) (oldarg, newarg); + DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); + } + if (DECL_TEMPLATE_INSTANTIATION (olddecl) && !DECL_TEMPLATE_INSTANTIATION (newdecl)) { @@ -8912,7 +8942,8 @@ grokfndecl (tree ctype, template_count, 2 * funcdef_flag + 4 * (friendp != 0) + - 8 * concept_p); + 8 * concept_p, + *attrlist); if (decl == error_mark_node) return NULL_TREE; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index ca73bb1..3bd44ab 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. If not see all methods must be provided in header files; can't use a source file that contains only the method templates and "just win". */ +#include <string> #include "config.h" #include "system.h" #include "coretypes.h" @@ -1473,8 +1474,10 @@ is_specialization_of_friend (tree decl, tree friend_decl) /* Register the specialization SPEC as a specialization of TMPL with the indicated ARGS. IS_FRIEND indicates whether the specialization - is actually just a friend declaration. Returns SPEC, or an - equivalent prior declaration, if available. + is actually just a friend declaration. ATTRLIST is the list of + attributes that the specialization is declared with or NULL when + it isn't. Returns SPEC, or an equivalent prior declaration, if + available. We also store instantiations of field packs in the hash table, even though they are not themselves templates, to make lookup easier. */ @@ -2610,6 +2613,110 @@ check_unqualified_spec_or_inst (tree t, location_t loc) } } +/* Warn for a template specialization SPEC that is missing some of a set + of function or type attributes that the template TEMPL is declared with. + ATTRLIST is a list of additional attributes that SPEC should be taken + to ultimately be declared with. */ + +static void +warn_spec_missing_attributes (tree tmpl, tree spec, tree attrlist) +{ + if (DECL_FUNCTION_TEMPLATE_P (tmpl)) + tmpl = DECL_TEMPLATE_RESULT (tmpl); + + if (TREE_CODE (tmpl) != FUNCTION_DECL) + return; + + /* Avoid warning if either declaration or its type is deprecated. */ + if (TREE_DEPRECATED (tmpl) + || TREE_DEPRECATED (spec)) + return; + + tree tmpl_type = TREE_TYPE (tmpl); + tree spec_type = TREE_TYPE (spec); + + if (TREE_DEPRECATED (tmpl_type) + || TREE_DEPRECATED (spec_type) + || TREE_DEPRECATED (TREE_TYPE (tmpl_type)) + || TREE_DEPRECATED (TREE_TYPE (spec_type))) + return; + + tree tmpl_attrs[] = { DECL_ATTRIBUTES (tmpl), TYPE_ATTRIBUTES (tmpl_type) }; + tree spec_attrs[] = { DECL_ATTRIBUTES (spec), TYPE_ATTRIBUTES (spec_type) }; + + if (!spec_attrs[0]) + spec_attrs[0] = attrlist; + else if (!spec_attrs[1]) + spec_attrs[1] = attrlist; + + /* Avoid warning if the primary has no attributes. */ + if (!tmpl_attrs[0] && !tmpl_attrs[1]) + return; + + /* Avoid warning if either declaration contains an attribute on + the white list below. */ + const char* const whitelist[] = { + "error", "noreturn", "warning" + }; + + for (unsigned i = 0; i != 2; ++i) + for (unsigned j = 0; j != sizeof whitelist / sizeof *whitelist; ++j) + if (lookup_attribute (whitelist[j], tmpl_attrs[i]) + || lookup_attribute (whitelist[j], spec_attrs[i])) + return; + + /* Avoid warning if the difference between the primary and + the specialization is not in one of the attributes below. */ + const char* const blacklist[] = { + "alloc_align", "alloc_size", "assume_aligned", "format", + "format_arg", "malloc", "nonnull", "warn_unused_result" + }; + + /* Put together a list of the black listed attributes that the primary + template is declared with that the specialization is not, in case + it's not apparent from the most recent declaration of the primary. */ + unsigned nattrs = 0; + std::string str; + + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) + { + for (unsigned j = 0; j != 2; ++j) + { + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) + continue; + + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) + { + if (lookup_attribute (blacklist[i], spec_attrs[k])) + break; + + if (str.size ()) + str += ", "; + str += "%<"; + str += blacklist[i]; + str += "%>"; + ++nattrs; + } + } + } + + if (!nattrs) + return; + + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attribute, + "explicit specialization %q#D may be missing attributes", + spec)) + { + if (nattrs > 1) + str = G_("missing primary template attributes ") + str; + else + str = G_("missing primary template attribute ") + str; + + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); + } + +} + /* Check to see if the function just declared, as indicated in DECLARATOR, and in DECL, is a specialization of a function template. We may also discover that the declaration is an explicit @@ -2651,7 +2758,8 @@ tree check_explicit_specialization (tree declarator, tree decl, int template_count, - int flags) + int flags, + tree attrlist) { int have_def = flags & 2; int is_friend = flags & 4; @@ -3108,8 +3216,13 @@ check_explicit_specialization (tree declarator, it again. Partial specializations will be registered in process_partial_specialization. */ if (!processing_template_decl) - decl = register_specialization (decl, gen_tmpl, targs, - is_friend, 0); + { + warn_spec_missing_attributes (gen_tmpl, decl, attrlist); + + decl = register_specialization (decl, gen_tmpl, targs, + is_friend, 0); + } + /* A 'structor should already have clones. */ gcc_assert (decl == error_mark_node diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f3d9336..f50e3f3 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -295,7 +295,7 @@ Objective-C and Objective-C++ Dialects}. -Winvalid-pch -Wlarger-than=@var{len} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol --Wmisleading-indentation -Wmissing-braces @gol +-Wmisleading-indentation -Wmissing-attribute -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-include-dirs @gol -Wno-multichar -Wmultistatement-macros -Wnonnull -Wnonnull-compare @gol -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol @@ -3887,6 +3887,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. -Wmemset-elt-size @gol -Wmemset-transposed-args @gol -Wmisleading-indentation @r{(only for C/C++)} @gol +-Wmissing-attribute @gol -Wmissing-braces @r{(only for C/ObjC)} @gol -Wmultistatement-macros @gol -Wnarrowing @r{(only for C++)} @gol @@ -4550,6 +4551,17 @@ about the layout of the file that the directive references. This warning is enabled by @option{-Wall} in C and C++. +@item -Wmissing-attribute +@opindex Wmissing-attribute +@opindex Wno-missing-attribute +Warn when a declaration of an entity is missing attributes that a related +entity is declared with whose absence may adversely affect the correctness +of generated code. For example, in C++, the warning is issued when +an explicit specialization of a primary template declared with attribute +@code{malloc} is declared without the attribute. + +This warning is enabled by @option{-Wall}. + @item -Wmissing-braces @opindex Wmissing-braces @opindex Wno-missing-braces diff --git a/gcc/testsuite/g++.dg/Wmissing-attribute.C b/gcc/testsuite/g++.dg/Wmissing-attribute.C new file mode 100644 index 0000000..5ce97cc --- /dev/null +++ b/gcc/testsuite/g++.dg/Wmissing-attribute.C @@ -0,0 +1,122 @@ +// PR c++/83871 - wrong code for attribute const and pure on distinct +// template specializations +// Test to verify that a declaration of an explicit specialization with +// no attributes is diagnosed when the primary template is declared with +// one or more attributes. The warning helps highlight a change in GCC +// 8 from previous versions that copied the attributes from the primary +// to the specialization. It also helps point out simply forgetting to +// declare the specialization with an attribute. +// { dg-do compile } +// { dg-options "-Wmissing-attribute" } + +#define ATTR(list) __attribute__ (list) + + +// Verify that a primary without attributes doesn't cause warnings. +template <class T> void fnoattr (); + +template <> void fnoattr<void>(); +template <> void ATTR ((cold)) fnoattr<int>(); +template <> void ATTR ((hot)) fnoattr<double>(); + +// Verify that a noreturn primary also doesn't cause warnings. +template <class T> int ATTR ((noreturn)) fnoreturn (); + +template <> int fnoreturn<void>(); +template <> int ATTR ((cold)) fnoreturn<int>(); +template <> int ATTR ((hot)) fnoreturn<double>(); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_all (int); // { dg-message "missing primary template attributes \(.malloc., .alloc_size.|.alloc_size., .malloc.\)" } + +template <> +void* +missing_all<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +// Verify that specifying the same attributes in whatever order +// doesn't trigger the warning, even when other attributes are +// added. +template <> +void* +ATTR ((alloc_size (1), malloc)) +missing_all<char>(int); + +template <> +void* +ATTR ((alloc_size (1))) ATTR ((malloc)) ATTR ((returns_nonnull)) +missing_all<char>(int); // T = char, same as above + +template <> +void* +ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc)) +missing_all<char>(int); // T = char, same as above + +// Verify that the following attributes suppress the warning. +template <> void* ATTR ((error (""))) missing_all<short>(int); +template <> void* ATTR ((deprecated)) missing_all<int>(int); +template <> void* ATTR ((noreturn)) missing_all<long>(int); +template <> void* ATTR ((warning (""))) missing_all<double>(int); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_malloc (int); // { dg-message "missing primary template attribute .malloc." } + +template <> +void* +ATTR ((alloc_size (1))) +missing_malloc<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((malloc, alloc_size (1))) missing_malloc<short>(int); +template <> void* ATTR ((deprecated)) missing_malloc<int>(int); +template <> void* ATTR ((error (""))) missing_malloc<long>(int); +template <> void* ATTR ((noreturn)) missing_malloc<float>(int); +template <> void* ATTR ((warning (""))) missing_malloc<double>(int); + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_alloc_size (int, int); // { dg-message "missing primary template attribute .alloc_size." } + +template <> +void* +ATTR ((malloc)) +missing_alloc_size<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + + +template <class T> +void* +ATTR ((nonnull (1))) +missing_nonnull (void*); // { dg-message "missing primary template attribute .nonnull." } + +template <> +void* +ATTR ((malloc)) +missing_nonnull<char>(void*); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((nonnull (1))) missing_nonnull<short>(void*); +template <> void* ATTR ((deprecated)) missing_nonnull<int>(void*); +template <> void* ATTR ((error (""))) missing_nonnull<long>(void*); +template <> void* ATTR ((noreturn)) missing_nonnull<float>(void*); +template <> void* ATTR ((warning (""))) missing_nonnull<double>(void*); + + +template <class T> +void* +ATTR ((warn_unused_result)) +missing_wur (); // { dg-message "missing primary template attribute .warn_unused_result." } + +template <> +void* +ATTR ((returns_nonnull)) +missing_wur<char>(); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((warn_unused_result)) missing_wur<short>(); +template <> void* ATTR ((deprecated)) missing_wur<int>(); +template <> void* ATTR ((error (""))) missing_wur<long>(); +template <> void* ATTR ((noreturn)) missing_wur<float>(); +template <> void* ATTR ((warning (""))) missing_wur<double>(); diff --git a/gcc/testsuite/g++.dg/ext/attr-const-pure.C b/gcc/testsuite/g++.dg/ext/attr-const-pure.C new file mode 100644 index 0000000..6afef6d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const-pure.C @@ -0,0 +1,103 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +int __attribute__ ((pure)) +f (T); + +template <> +int __attribute__ ((const)) f<int> (int); // { dg-bogus "ignoring attribute .const." } + + +template <class T> +int __attribute__ ((const)) +g (T); + +template <> +int __attribute__ ((pure)) g<int> (int); // { dg-bogus "ignoring attribute .const." } + +template <class T> +int __attribute__ ((const)) +h (T); + +template <class T> +int __attribute__ ((pure)) +h (const T*); + +template <> +int h<int> (int); + +template <> +int h<int*> (int*); + +int global; + +extern void h_primary_elim (); +extern void h_cstptr_elim (); +extern void h_cstptr_keep (); +extern void h_int_keep (); +extern void h_intptr_keep (); + +void call_primary_elim (double x) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. + int i0 = h (x); + int i1 = h (x); + + if (i0 != i1) // must be folded into false + h_primary_elim (); // must be eliminated +} + +void call_cstptr_elim (const void *p) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. This verifies that h<const + // void*>*() is treated as const in this context. + int i0 = h (p); + int i1 = h (p); + + if (i0 != i1) // must be folded into false + h_cstptr_elim (); // must be eliminated +} + +void call_cstptr_keep (const void *p) +{ + // Because of the store to the global, both calls to h(p) must + // be emitted. This verifies that h<const void*>*() is not + // treated as const. + int i0 = h (p); + global = 123; + int i1 = h (p); + + if (i0 != i1) // must not be folded + h_cstptr_keep (); // must be emitted +} + +void call_int_keep (int i) +{ + // Both calls to h(i) must be emitted. + int i0 = h (i); + int i1 = h (i); + + if (i0 != i1) // must not be folded + h_int_keep (); // must be emitted +} + +void call_intptr_keep (int *ip) +{ + // Both calls to h(ip) must be emitted. + int i0 = h (ip); + int i1 = h (ip); + + if (i0 != i1) // must not be folded + h_intptr_keep (); // must be emitted +} + +// { dg-final { scan-tree-dump-not "h_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "h_cstptr_elim" "optimized" } } +// { dg-final { scan-tree-dump-times "h_cstptr_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_int_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_intptr_keep" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc.C b/gcc/testsuite/g++.dg/ext/attr-malloc.C new file mode 100644 index 0000000..ce76abe --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc.C @@ -0,0 +1,45 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that an explicit template specifialization does not +// "inherit" attribute malloc from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((malloc)) +f (unsigned); + +template <> +void* +f<int>(unsigned); // { dg-warning "may be missing attributes" } + +static char a[8]; + +void f_void_malloc (); +void f_int_not_malloc (); + +void fv (void) +{ + void *p = f<void>(1); + if (!p) + return; + + if (p == a) // must be false + f_void_malloc (); // should be eliminated +} + + +void fi (void) +{ + void *p = f<int>(1); + if (!p) + return; + + if (p == a) // can be true + f_int_not_malloc (); // must not be eliminated +} + +// Verify that the call to f_void_not_malloc() is eliminated but +// the call to f_int_not_malloc() is retained. +// { dg-final { scan-tree-dump-not "f_void_malloc" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_not_malloc" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-nonnull.C new file mode 100644 index 0000000..57d2cb0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nonnull.C @@ -0,0 +1,31 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nonnull (1))) +f (T*, T*, T*); + +template <> +void +f<int>(int*, int*, int*); // { dg-warning "may be missing attributes" } + +template <> +void __attribute__ ((nonnull (3))) +f<float>(float*, float*, float*); + + +void test_nonnull (void) +{ + f<void>(0, 0, 0); // { dg-warning "null argument where non-null required \\\(argument 1\\\)" } + + f<int>(0, 0, 0); // { dg-bogus "null argument" } + + f<float>(0, 0, 0); + // { dg-bogus "null argument where non-null required \\\(argument 1\\\)" "" { target *-*-* } .-1 } + // { dg-warning "null argument where non-null required \\\(argument 3\\\)" "" { target *-*-* } .-2 } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C new file mode 100644 index 0000000..4061f82 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C @@ -0,0 +1,36 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + { dg-do compile } + { dg-options "-O -fdump-tree-eh" } */ + +void __attribute__ ((nothrow)) f (); + +void ff () throw () +{ + // No exception handling necessary around the call to f(). + f (); +} + +void __attribute__ ((nothrow)) g (); +void g (); + +void gg () throw () +{ + // No exception handling necessary around the call to g(). + g (); +} + +int __attribute__ ((nothrow)) h (); +int __attribute__ ((noreturn)) h (); +int h (); + +int hh () throw () +{ + // No exception handling necessary around the call to h(). + // No -Wreturn-value should be emitted because h is noreturn. + h (); +} + +// Verify that no exception handling code was emitted. +// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } } +// { dg-final { scan-tree-dump-not "resx" "eh" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow.C b/gcc/testsuite/g++.dg/ext/attr-nothrow.C new file mode 100644 index 0000000..ae6b674 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow.C @@ -0,0 +1,47 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute nothrow from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nothrow)) +f (); + +template <> +void +f<int>(); + +void f_void_nothrow (); +void f_int_maythrow (); + +void fv (void) +{ + try + { + f<void>(); + } + catch (...) // cannot be be reached + { + f_void_nothrow (); // should be eliminated + } +} + + +void fi (void) +{ + try + { + f<int>(); + } + catch (...) // may be reached + { + f_int_maythrow (); // must not be eliminated + } +} + +// Verify that the call to f_void_nothrow() is eliminated but +// the call to f_int_maythrow() is retained. +// { dg-final { scan-tree-dump-not "f_void_nothrow" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_maythrow" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C new file mode 100644 index 0000000..f75f32e --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C @@ -0,0 +1,42 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((returns_nonnull)) +g (); + +template <> +void* +g<int>(); + +extern void g_void_returns_nonnull (); +extern void g_int_may_return_null (); + +void test_returns_nonnull () +{ + void *p = g<void>(); + if (!p) + g_void_returns_nonnull (); + + (void)&p; +} + +void test_may_return_null () +{ + void *p = g<int>(); + if (!p) + g_int_may_return_null (); + + (void)&p; +} + + +// Verify that the call to g_void_returns_nonnull() is eliminated but +// the call to g_int_may_return_null() is retained. +// { dg-final { scan-tree-dump-not "g_void_returns_nonnull" "optimized" } } +// { dg-final { scan-tree-dump-times "g_int_may_return_null" 1 "optimized" } }
On 02/08/2018 04:52 PM, Martin Sebor wrote: > I took me a while to find DECL_TEMPLATE_RESULT. Hopefully > that's the right way to get the primary from a TEMPLATE_DECL. Yes. >>> Attached is an updated patch. It hasn't gone through full >>> testing yet but please let me know if you'd like me to make >>> some changes. >> >>> + const char* const whitelist[] = { >>> + "error", "noreturn", "warning" >>> + }; >> >> Why whitelist noreturn? I would expect to want that to be consistent. > > I expect noreturn to be used on a primary whose definition > is provided but that's not meant to be used the way the API > is otherwise expected to be. As in: > > template <class T> > T [[noreturn]] foo () { throw "not implemented"; } > > template <> int foo<int>(); // implemented elsewhere Marking that template as noreturn seems pointless, and possibly harmful; the deprecated, warning, or error attributes would be better for this situation. > - /* Merge the type qualifiers. */ > - if (TREE_READONLY (newdecl)) > - TREE_READONLY (olddecl) = 1; > if (TREE_THIS_VOLATILE (newdecl)) > TREE_THIS_VOLATILE (olddecl) = 1; > - if (TREE_NOTHROW (newdecl)) > - TREE_NOTHROW (olddecl) = 1; > + > + if (merge_attr) > + { > + /* Merge the type qualifiers. */ > + if (TREE_READONLY (newdecl)) > + TREE_READONLY (olddecl) = 1; > + } > + else > + { > + /* Set the bits that correspond to the const function attributes. */ > + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); > + } Let's limit the const/volatile handling here to non-functions, and handle the const/noreturn attributes for functions in the later hunk along with nothrow/malloc/pure. > - /* Merge parameter attributes. */ > + /* Merge or assign parameter attributes. */ > tree oldarg, newarg; > - for (oldarg = DECL_ARGUMENTS(olddecl), > - newarg = DECL_ARGUMENTS(newdecl); > - oldarg && newarg; > - oldarg = DECL_CHAIN(oldarg), newarg = DECL_CHAIN(newarg)) { > - DECL_ATTRIBUTES (newarg) > - = (*targetm.merge_decl_attributes) (oldarg, newarg); > - DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); > - } > - > + for (oldarg = DECL_ARGUMENTS (olddecl), > + newarg = DECL_ARGUMENTS (newdecl); > + oldarg && newarg; > + oldarg = DECL_CHAIN (oldarg), newarg = DECL_CHAIN (newarg)) > + { > + DECL_ATTRIBUTES (newarg) > + = (*targetm.merge_decl_attributes) (oldarg, newarg); > + DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); > + } > + I try to avoid reformatting code that I'm not actually changing, to avoid noise in e.g. git blame. Jason
On 02/09/2018 12:52 PM, Jason Merrill wrote: > On 02/08/2018 04:52 PM, Martin Sebor wrote: >> I took me a while to find DECL_TEMPLATE_RESULT. Hopefully >> that's the right way to get the primary from a TEMPLATE_DECL. > > Yes. > >>>> Attached is an updated patch. It hasn't gone through full >>>> testing yet but please let me know if you'd like me to make >>>> some changes. >>> >>>> + const char* const whitelist[] = { >>>> + "error", "noreturn", "warning" >>>> + }; >>> >>> Why whitelist noreturn? I would expect to want that to be consistent. >> >> I expect noreturn to be used on a primary whose definition >> is provided but that's not meant to be used the way the API >> is otherwise expected to be. As in: >> >> template <class T> >> T [[noreturn]] foo () { throw "not implemented"; } >> >> template <> int foo<int>(); // implemented elsewhere > > Marking that template as noreturn seems pointless, and possibly harmful; > the deprecated, warning, or error attributes would be better for this > situation. I meant either: template <class T> T __attribute__ ((noreturn)) foo () { throw "not implemented"; } template <> int foo<int>(); // implemented elsewhere or (sigh) template <class T> [[noreturn]] T foo () { throw "not implemented"; } template <> int foo<int>(); // implemented elsewhere It lets code like this int bar () { return foo<char>(); } be diagnosed because it's likely a bug (as Clang does with -Wunreachable-code). It doesn't stop code like the following from compiling (which is good) but it instead lets them throw at runtime which is what foo's author wants. void bar () { foo<char>(); } It's the same as having an "unimplemented" base virtual function throw an exception when it's called rather than making it pure and having calls to it abort. Declaring the base virtual function noreturn is useful for the same reason (and also diagnosed by Clang). I should remember to add the same warning in GCC 9. I actually had some misgivings about both warning and deprecated for the white-listing, but not for noreturn. My (only mild) concern is that both warning and deprecated functions can and likely will in some cases still be called, and so using them to suppress the warning runs the risk that their calls might be wrong and no one will notice. Warning cannot be suppressed so it seems unlikely to be ignored, but deprecated can be. So I wonder if the white-listing for deprecated should be conditional on -Wdeprecated being enabled. >> - /* Merge the type qualifiers. */ >> - if (TREE_READONLY (newdecl)) >> - TREE_READONLY (olddecl) = 1; >> if (TREE_THIS_VOLATILE (newdecl)) >> TREE_THIS_VOLATILE (olddecl) = 1; >> - if (TREE_NOTHROW (newdecl)) >> - TREE_NOTHROW (olddecl) = 1; >> + >> + if (merge_attr) >> + { >> + /* Merge the type qualifiers. */ >> + if (TREE_READONLY (newdecl)) >> + TREE_READONLY (olddecl) = 1; >> + } >> + else >> + { >> + /* Set the bits that correspond to the const function >> attributes. */ >> + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); >> + } > > Let's limit the const/volatile handling here to non-functions, and > handle the const/noreturn attributes for functions in the later hunk > along with nothrow/malloc/pure. I had to keep the TREE_HAS_VOLATILE handling as is since it applies to functions too (has side-effects). Otherwise the attr-nothrow-2.C test fails. > > I try to avoid reformatting code that I'm not actually changing, to > avoid noise in e.g. git blame. Okay. Attached is an updated patch. Besides the changes above I also used a plural "attributes" in the name of the option for consistency with other attribute options, expanded the documentation, and enhanced one of the tests. Bootstrapped on x86_64-linux (tests still running). Martin PR c++/83871 - wrong code for attribute const and pure on distinct template specializations PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization gcc/ChangeLog: PR c++/83871 * gcc/doc/invoke.texi (-Wmissing-attribute): New option. gcc/c-family/ChangeLog: PR c++/83871 * c.opt (-Wmissing-attribute): New option. gcc/cp/ChangeLog: PR c++/83871 PR c++/83503 * cp-tree.h (warn_spec_missing_attributes): New function. ((check_explicit_specialization): Add an argument. Call the above function. * decl.c (duplicate_decls): Avoid applying primary function template's attributes to its explicit specializations. gcc/testsuite/ChangeLog: PR c++/83871 PR c++/83503 * g++.dg/ext/attr-const-pure.C: New test. * g++.dg/ext/attr-malloc.C: New test. * g++.dg/ext/attr-nonnull.C: New test. * g++.dg/ext/attr-nothrow.C: New test. * g++.dg/ext/attr-nothrow-2.C: New test. * g++.dg/ext/attr-returns-nonnull.C: New test. * g++.dg/Wmissing-attribute.C: New test. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9c71726..1039274 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -781,6 +781,11 @@ Wtemplates C++ ObjC++ Var(warn_templates) Warning Warn on primary template declaration. +Wmissing-attributes +C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn about declarations of entities that may be missing attributes +that related entities have been declared with it. + Wmissing-format-attribute C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format) ; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a53f4fd..87b7916 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6470,7 +6470,8 @@ extern void end_specialization (void); extern void begin_explicit_instantiation (void); extern void end_explicit_instantiation (void); extern void check_unqualified_spec_or_inst (tree, location_t); -extern tree check_explicit_specialization (tree, tree, int, int); +extern tree check_explicit_specialization (tree, tree, int, int, + tree = NULL_TREE); extern int num_template_headers_for_class (tree); extern void check_template_variable (tree); extern tree make_auto (void); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 244a3ef..2ea115e 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -1971,10 +1971,21 @@ next_arg:; DECL_ORIGINAL_TYPE (newdecl) = DECL_ORIGINAL_TYPE (olddecl); } - /* Copy all the DECL_... slots specified in the new decl - except for any that we copy here from the old type. */ - DECL_ATTRIBUTES (newdecl) - = (*targetm.merge_decl_attributes) (olddecl, newdecl); + /* True to merge attributes between the declarations, false to + set OLDDECL's attributes to those of NEWDECL (for template + explicit specializations that specify their own attributes + independent of those specified for the primary template). */ + const bool merge_attr = (TREE_CODE (newdecl) != FUNCTION_DECL + || !DECL_TEMPLATE_SPECIALIZATION (newdecl) + || DECL_TEMPLATE_SPECIALIZATION (olddecl)); + + /* Copy all the DECL_... slots specified in the new decl except for + any that we copy here from the old type. */ + if (merge_attr) + DECL_ATTRIBUTES (newdecl) + = (*targetm.merge_decl_attributes) (olddecl, newdecl); + else + DECL_ATTRIBUTES (olddecl) = DECL_ATTRIBUTES (newdecl); if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl)) { @@ -2101,9 +2112,10 @@ next_arg:; } } } - else - /* Merge the data types specified in the two decls. */ + else if (merge_attr) newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); + else + newtype = TREE_TYPE (newdecl); if (VAR_P (newdecl)) { @@ -2167,14 +2179,6 @@ next_arg:; && !(processing_template_decl && uses_template_parms (newdecl))) layout_decl (newdecl, 0); - /* Merge the type qualifiers. */ - if (TREE_READONLY (newdecl)) - TREE_READONLY (olddecl) = 1; - if (TREE_THIS_VOLATILE (newdecl)) - TREE_THIS_VOLATILE (olddecl) = 1; - if (TREE_NOTHROW (newdecl)) - TREE_NOTHROW (olddecl) = 1; - /* Merge deprecatedness. */ if (TREE_DEPRECATED (newdecl)) TREE_DEPRECATED (olddecl) = 1; @@ -2192,6 +2196,17 @@ next_arg:; DECL_FUNCTION_SPECIFIC_OPTIMIZATION (newdecl) = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (olddecl); } + else + { + /* Merge the const type qualifier. */ + if (TREE_READONLY (newdecl)) + TREE_READONLY (olddecl) = 1; + } + + /* Merge the volatile type qualifier and the function has + side-effects bit. */ + if (TREE_THIS_VOLATILE (newdecl)) + TREE_THIS_VOLATILE (olddecl) = 1; /* Merge the initialization information. */ if (DECL_INITIAL (newdecl) == NULL_TREE @@ -2212,13 +2227,24 @@ next_arg:; |= DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (olddecl); DECL_NO_LIMIT_STACK (newdecl) |= DECL_NO_LIMIT_STACK (olddecl); TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); - TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); - DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); - DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); - TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); - DECL_LOOPING_CONST_OR_PURE_P (newdecl) + DECL_LOOPING_CONST_OR_PURE_P (newdecl) |= DECL_LOOPING_CONST_OR_PURE_P (olddecl); + + if (merge_attr) + { + TREE_NOTHROW (olddecl) |= TREE_NOTHROW (newdecl); + TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); + DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); + DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); + } + else + { + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); + DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl); + } /* Keep the old RTL. */ COPY_DECL_RTL (olddecl, newdecl); } @@ -8912,7 +8938,8 @@ grokfndecl (tree ctype, template_count, 2 * funcdef_flag + 4 * (friendp != 0) + - 8 * concept_p); + 8 * concept_p, + *attrlist); if (decl == error_mark_node) return NULL_TREE; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index ca73bb1..dfdb3eb 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. If not see all methods must be provided in header files; can't use a source file that contains only the method templates and "just win". */ +#include <string> #include "config.h" #include "system.h" #include "coretypes.h" @@ -1473,8 +1474,10 @@ is_specialization_of_friend (tree decl, tree friend_decl) /* Register the specialization SPEC as a specialization of TMPL with the indicated ARGS. IS_FRIEND indicates whether the specialization - is actually just a friend declaration. Returns SPEC, or an - equivalent prior declaration, if available. + is actually just a friend declaration. ATTRLIST is the list of + attributes that the specialization is declared with or NULL when + it isn't. Returns SPEC, or an equivalent prior declaration, if + available. We also store instantiations of field packs in the hash table, even though they are not themselves templates, to make lookup easier. */ @@ -2610,6 +2613,110 @@ check_unqualified_spec_or_inst (tree t, location_t loc) } } +/* Warn for a template specialization SPEC that is missing some of a set + of function or type attributes that the template TEMPL is declared with. + ATTRLIST is a list of additional attributes that SPEC should be taken + to ultimately be declared with. */ + +static void +warn_spec_missing_attributes (tree tmpl, tree spec, tree attrlist) +{ + if (DECL_FUNCTION_TEMPLATE_P (tmpl)) + tmpl = DECL_TEMPLATE_RESULT (tmpl); + + if (TREE_CODE (tmpl) != FUNCTION_DECL) + return; + + /* Avoid warning if either declaration or its type is deprecated. */ + if (TREE_DEPRECATED (tmpl) + || TREE_DEPRECATED (spec)) + return; + + tree tmpl_type = TREE_TYPE (tmpl); + tree spec_type = TREE_TYPE (spec); + + if (TREE_DEPRECATED (tmpl_type) + || TREE_DEPRECATED (spec_type) + || TREE_DEPRECATED (TREE_TYPE (tmpl_type)) + || TREE_DEPRECATED (TREE_TYPE (spec_type))) + return; + + tree tmpl_attrs[] = { DECL_ATTRIBUTES (tmpl), TYPE_ATTRIBUTES (tmpl_type) }; + tree spec_attrs[] = { DECL_ATTRIBUTES (spec), TYPE_ATTRIBUTES (spec_type) }; + + if (!spec_attrs[0]) + spec_attrs[0] = attrlist; + else if (!spec_attrs[1]) + spec_attrs[1] = attrlist; + + /* Avoid warning if the primary has no attributes. */ + if (!tmpl_attrs[0] && !tmpl_attrs[1]) + return; + + /* Avoid warning if either declaration contains an attribute on + the white list below. */ + const char* const whitelist[] = { + "error", "noreturn", "warning" + }; + + for (unsigned i = 0; i != 2; ++i) + for (unsigned j = 0; j != sizeof whitelist / sizeof *whitelist; ++j) + if (lookup_attribute (whitelist[j], tmpl_attrs[i]) + || lookup_attribute (whitelist[j], spec_attrs[i])) + return; + + /* Avoid warning if the difference between the primary and + the specialization is not in one of the attributes below. */ + const char* const blacklist[] = { + "alloc_align", "alloc_size", "assume_aligned", "format", + "format_arg", "malloc", "nonnull", "warn_unused_result" + }; + + /* Put together a list of the black listed attributes that the primary + template is declared with that the specialization is not, in case + it's not apparent from the most recent declaration of the primary. */ + unsigned nattrs = 0; + std::string str; + + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) + { + for (unsigned j = 0; j != 2; ++j) + { + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) + continue; + + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) + { + if (lookup_attribute (blacklist[i], spec_attrs[k])) + break; + + if (str.size ()) + str += ", "; + str += "%<"; + str += blacklist[i]; + str += "%>"; + ++nattrs; + } + } + } + + if (!nattrs) + return; + + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attributes, + "explicit specialization %q#D may be missing attributes", + spec)) + { + if (nattrs > 1) + str = G_("missing primary template attributes ") + str; + else + str = G_("missing primary template attribute ") + str; + + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); + } + +} + /* Check to see if the function just declared, as indicated in DECLARATOR, and in DECL, is a specialization of a function template. We may also discover that the declaration is an explicit @@ -2651,7 +2758,8 @@ tree check_explicit_specialization (tree declarator, tree decl, int template_count, - int flags) + int flags, + tree attrlist) { int have_def = flags & 2; int is_friend = flags & 4; @@ -3108,8 +3216,13 @@ check_explicit_specialization (tree declarator, it again. Partial specializations will be registered in process_partial_specialization. */ if (!processing_template_decl) - decl = register_specialization (decl, gen_tmpl, targs, - is_friend, 0); + { + warn_spec_missing_attributes (gen_tmpl, decl, attrlist); + + decl = register_specialization (decl, gen_tmpl, targs, + is_friend, 0); + } + /* A 'structor should already have clones. */ gcc_assert (decl == error_mark_node diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f3d9336..4208647 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -295,7 +295,7 @@ Objective-C and Objective-C++ Dialects}. -Winvalid-pch -Wlarger-than=@var{len} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol --Wmisleading-indentation -Wmissing-braces @gol +-Wmisleading-indentation -Wmissing-attributes -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-include-dirs @gol -Wno-multichar -Wmultistatement-macros -Wnonnull -Wnonnull-compare @gol -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol @@ -3887,6 +3887,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. -Wmemset-elt-size @gol -Wmemset-transposed-args @gol -Wmisleading-indentation @r{(only for C/C++)} @gol +-Wmissing-attributes @gol -Wmissing-braces @r{(only for C/ObjC)} @gol -Wmultistatement-macros @gol -Wnarrowing @r{(only for C++)} @gol @@ -4550,6 +4551,36 @@ about the layout of the file that the directive references. This warning is enabled by @option{-Wall} in C and C++. +@item -Wmissing-attributes +@opindex Wmissing-attributes +@opindex Wno-missing-attributes +Warn when a declaration of a function is missing one or more attributes +that a related function is declared with and whose absence may adversely +affect the correctness or efficiency of generated code. For example, in +C++, the warning is issued when an explicit specialization of a primary +template declared with attribute @code{alloc_align}, @code{alloc_size}, +@code{assume_aligned}, @code{format}, @code{format_arg}, @code{malloc}, +@code{nonnull}, or @code{warn_unused_result} is declared without it. +Attributes @code{deprecated}, @code{error}, @code{noreturn}, and +@code{warning} suppress the warning. (@pxref{Function Attributes}). + +@option{-Wmissing-attributes} is enabled by @option{-Wall}. + +For example, since the declaration of the primary function template +below makes use of both attribute @code{malloc} and @code{alloc_size} +the declaration of the explicit specialization of the template is +diagnosed because it is missing one of the attributes. + +@smallexample +template <class T> +T* __attribute__ ((malloc, alloc_size (1))) +allocate (size_t); + +template <> +void* __attribute__ ((malloc)) // missing alloc_size +allocate<void> (size_t); +@end smallexample + @item -Wmissing-braces @opindex Wmissing-braces @opindex Wno-missing-braces diff --git a/gcc/testsuite/g++.dg/Wmissing-attribute.C b/gcc/testsuite/g++.dg/Wmissing-attribute.C new file mode 100644 index 0000000..5ce97cc --- /dev/null +++ b/gcc/testsuite/g++.dg/Wmissing-attribute.C @@ -0,0 +1,122 @@ +// PR c++/83871 - wrong code for attribute const and pure on distinct +// template specializations +// Test to verify that a declaration of an explicit specialization with +// no attributes is diagnosed when the primary template is declared with +// one or more attributes. The warning helps highlight a change in GCC +// 8 from previous versions that copied the attributes from the primary +// to the specialization. It also helps point out simply forgetting to +// declare the specialization with an attribute. +// { dg-do compile } +// { dg-options "-Wmissing-attribute" } + +#define ATTR(list) __attribute__ (list) + + +// Verify that a primary without attributes doesn't cause warnings. +template <class T> void fnoattr (); + +template <> void fnoattr<void>(); +template <> void ATTR ((cold)) fnoattr<int>(); +template <> void ATTR ((hot)) fnoattr<double>(); + +// Verify that a noreturn primary also doesn't cause warnings. +template <class T> int ATTR ((noreturn)) fnoreturn (); + +template <> int fnoreturn<void>(); +template <> int ATTR ((cold)) fnoreturn<int>(); +template <> int ATTR ((hot)) fnoreturn<double>(); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_all (int); // { dg-message "missing primary template attributes \(.malloc., .alloc_size.|.alloc_size., .malloc.\)" } + +template <> +void* +missing_all<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +// Verify that specifying the same attributes in whatever order +// doesn't trigger the warning, even when other attributes are +// added. +template <> +void* +ATTR ((alloc_size (1), malloc)) +missing_all<char>(int); + +template <> +void* +ATTR ((alloc_size (1))) ATTR ((malloc)) ATTR ((returns_nonnull)) +missing_all<char>(int); // T = char, same as above + +template <> +void* +ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc)) +missing_all<char>(int); // T = char, same as above + +// Verify that the following attributes suppress the warning. +template <> void* ATTR ((error (""))) missing_all<short>(int); +template <> void* ATTR ((deprecated)) missing_all<int>(int); +template <> void* ATTR ((noreturn)) missing_all<long>(int); +template <> void* ATTR ((warning (""))) missing_all<double>(int); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_malloc (int); // { dg-message "missing primary template attribute .malloc." } + +template <> +void* +ATTR ((alloc_size (1))) +missing_malloc<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((malloc, alloc_size (1))) missing_malloc<short>(int); +template <> void* ATTR ((deprecated)) missing_malloc<int>(int); +template <> void* ATTR ((error (""))) missing_malloc<long>(int); +template <> void* ATTR ((noreturn)) missing_malloc<float>(int); +template <> void* ATTR ((warning (""))) missing_malloc<double>(int); + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_alloc_size (int, int); // { dg-message "missing primary template attribute .alloc_size." } + +template <> +void* +ATTR ((malloc)) +missing_alloc_size<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + + +template <class T> +void* +ATTR ((nonnull (1))) +missing_nonnull (void*); // { dg-message "missing primary template attribute .nonnull." } + +template <> +void* +ATTR ((malloc)) +missing_nonnull<char>(void*); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((nonnull (1))) missing_nonnull<short>(void*); +template <> void* ATTR ((deprecated)) missing_nonnull<int>(void*); +template <> void* ATTR ((error (""))) missing_nonnull<long>(void*); +template <> void* ATTR ((noreturn)) missing_nonnull<float>(void*); +template <> void* ATTR ((warning (""))) missing_nonnull<double>(void*); + + +template <class T> +void* +ATTR ((warn_unused_result)) +missing_wur (); // { dg-message "missing primary template attribute .warn_unused_result." } + +template <> +void* +ATTR ((returns_nonnull)) +missing_wur<char>(); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((warn_unused_result)) missing_wur<short>(); +template <> void* ATTR ((deprecated)) missing_wur<int>(); +template <> void* ATTR ((error (""))) missing_wur<long>(); +template <> void* ATTR ((noreturn)) missing_wur<float>(); +template <> void* ATTR ((warning (""))) missing_wur<double>(); diff --git a/gcc/testsuite/g++.dg/ext/attr-const-pure.C b/gcc/testsuite/g++.dg/ext/attr-const-pure.C new file mode 100644 index 0000000..f7c6f3b --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const-pure.C @@ -0,0 +1,144 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +int global; + +template <class T> +int __attribute__ ((pure)) +f (T); + +template <> +int __attribute__ ((const)) f<int> (int); // { dg-bogus "ignoring attribute .const." } + +void f_pure_primary_elim (); +void f_pure_primary_keep (); +void f_const_spec_elim (); + +void call_pure_primary_elim (double x) +{ + // Only the first call to f(x) must be emitted, the second one + // is expected to be eliminated because the primary template + // is pure. + int i0 = f (x); + int i1 = f (x); + if (i0 != i1) + f_pure_primary_elim (); +} + +void call_pure_primary_keep (const char *s) +{ + // Both calls to f(x) must be emitted because the primary is + // pure and may read global. + int i0 = f (s); + global = 123; + int i1 = f (s); + if (i0 != i1) + f_pure_primary_keep (); +} + +void call_const_spec_elim (int i) +{ + // Only the first call to f(x) must be emitted, the second + // one is expected to be eliminated again, this time because + // unlike the pure primary, the specialization is const. + int i0 = f (i); + global = 123; + int i1 = f (i); + if (i0 != i1) + f_const_spec_elim (); +} + +template <class T> +int __attribute__ ((const)) +g (T); + +template <> +int __attribute__ ((pure)) g<int> (int); // { dg-bogus "ignoring attribute .const." } + +template <class T> +int __attribute__ ((const)) +h (T); + +template <class T> +int __attribute__ ((pure)) +h (const T*); + +template <> +int h<int> (int); + +template <> +int h<int*> (int*); + +extern void h_const_primary_elim (); +extern void h_pure_cstptr_elim (); +extern void h_cstptr_keep (); +extern void h_int_keep (); +extern void h_intptr_keep (); + +void call_const_primary_elim (double x) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. + int i0 = h (x); + int i1 = h (x); + + if (i0 != i1) // must be folded into false + h_const_primary_elim (); // must be eliminated +} + +void call_pure_cstptr_elim (const void *p) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. This verifies that h<const + // void*>*() is treated as const in this context. + int i0 = h (p); + int i1 = h (p); + + if (i0 != i1) // must be folded into false + h_pure_cstptr_elim (); // must be eliminated +} + +void call_cstptr_keep (const void *p) +{ + // Because of the store to the global, both calls to h(p) must + // be emitted. This verifies that h<const void*>*() is not + // treated as const. + int i0 = h (p); + global = 123; + int i1 = h (p); + + if (i0 != i1) // must not be folded + h_cstptr_keep (); // must be emitted +} + +void call_int_keep (int i) +{ + // Both calls to h(i) must be emitted. + int i0 = h (i); + int i1 = h (i); + + if (i0 != i1) // must not be folded + h_int_keep (); // must be emitted +} + +void call_intptr_keep (int *ip) +{ + // Both calls to h(ip) must be emitted. + int i0 = h (ip); + int i1 = h (ip); + + if (i0 != i1) // must not be folded + h_intptr_keep (); // must be emitted +} + +// { dg-final { scan-tree-dump-not "f_pure_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "f_const_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "f_const_spec_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "h_pure_cstptr_elim" "optimized" } } + +// { dg-final { scan-tree-dump-times "f_pure_primary_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_cstptr_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_int_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_intptr_keep" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc.C b/gcc/testsuite/g++.dg/ext/attr-malloc.C new file mode 100644 index 0000000..ce76abe --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc.C @@ -0,0 +1,45 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that an explicit template specifialization does not +// "inherit" attribute malloc from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((malloc)) +f (unsigned); + +template <> +void* +f<int>(unsigned); // { dg-warning "may be missing attributes" } + +static char a[8]; + +void f_void_malloc (); +void f_int_not_malloc (); + +void fv (void) +{ + void *p = f<void>(1); + if (!p) + return; + + if (p == a) // must be false + f_void_malloc (); // should be eliminated +} + + +void fi (void) +{ + void *p = f<int>(1); + if (!p) + return; + + if (p == a) // can be true + f_int_not_malloc (); // must not be eliminated +} + +// Verify that the call to f_void_not_malloc() is eliminated but +// the call to f_int_not_malloc() is retained. +// { dg-final { scan-tree-dump-not "f_void_malloc" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_not_malloc" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-nonnull.C new file mode 100644 index 0000000..57d2cb0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nonnull.C @@ -0,0 +1,31 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nonnull (1))) +f (T*, T*, T*); + +template <> +void +f<int>(int*, int*, int*); // { dg-warning "may be missing attributes" } + +template <> +void __attribute__ ((nonnull (3))) +f<float>(float*, float*, float*); + + +void test_nonnull (void) +{ + f<void>(0, 0, 0); // { dg-warning "null argument where non-null required \\\(argument 1\\\)" } + + f<int>(0, 0, 0); // { dg-bogus "null argument" } + + f<float>(0, 0, 0); + // { dg-bogus "null argument where non-null required \\\(argument 1\\\)" "" { target *-*-* } .-1 } + // { dg-warning "null argument where non-null required \\\(argument 3\\\)" "" { target *-*-* } .-2 } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C new file mode 100644 index 0000000..4061f82 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C @@ -0,0 +1,36 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + { dg-do compile } + { dg-options "-O -fdump-tree-eh" } */ + +void __attribute__ ((nothrow)) f (); + +void ff () throw () +{ + // No exception handling necessary around the call to f(). + f (); +} + +void __attribute__ ((nothrow)) g (); +void g (); + +void gg () throw () +{ + // No exception handling necessary around the call to g(). + g (); +} + +int __attribute__ ((nothrow)) h (); +int __attribute__ ((noreturn)) h (); +int h (); + +int hh () throw () +{ + // No exception handling necessary around the call to h(). + // No -Wreturn-value should be emitted because h is noreturn. + h (); +} + +// Verify that no exception handling code was emitted. +// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } } +// { dg-final { scan-tree-dump-not "resx" "eh" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow.C b/gcc/testsuite/g++.dg/ext/attr-nothrow.C new file mode 100644 index 0000000..ae6b674 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow.C @@ -0,0 +1,47 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute nothrow from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nothrow)) +f (); + +template <> +void +f<int>(); + +void f_void_nothrow (); +void f_int_maythrow (); + +void fv (void) +{ + try + { + f<void>(); + } + catch (...) // cannot be be reached + { + f_void_nothrow (); // should be eliminated + } +} + + +void fi (void) +{ + try + { + f<int>(); + } + catch (...) // may be reached + { + f_int_maythrow (); // must not be eliminated + } +} + +// Verify that the call to f_void_nothrow() is eliminated but +// the call to f_int_maythrow() is retained. +// { dg-final { scan-tree-dump-not "f_void_nothrow" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_maythrow" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C new file mode 100644 index 0000000..f75f32e --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C @@ -0,0 +1,42 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((returns_nonnull)) +g (); + +template <> +void* +g<int>(); + +extern void g_void_returns_nonnull (); +extern void g_int_may_return_null (); + +void test_returns_nonnull () +{ + void *p = g<void>(); + if (!p) + g_void_returns_nonnull (); + + (void)&p; +} + +void test_may_return_null () +{ + void *p = g<int>(); + if (!p) + g_int_may_return_null (); + + (void)&p; +} + + +// Verify that the call to g_void_returns_nonnull() is eliminated but +// the call to g_int_may_return_null() is retained. +// { dg-final { scan-tree-dump-not "g_void_returns_nonnull" "optimized" } } +// { dg-final { scan-tree-dump-times "g_int_may_return_null" 1 "optimized" } }
On Fri, Feb 9, 2018 at 6:57 PM, Martin Sebor <msebor@gmail.com> wrote: > On 02/09/2018 12:52 PM, Jason Merrill wrote: >> On 02/08/2018 04:52 PM, Martin Sebor wrote: >>> >>> I took me a while to find DECL_TEMPLATE_RESULT. Hopefully >>> that's the right way to get the primary from a TEMPLATE_DECL. >> >> Yes. >> >>>>> Attached is an updated patch. It hasn't gone through full >>>>> testing yet but please let me know if you'd like me to make >>>>> some changes. >>>> >>>> >>>>> + const char* const whitelist[] = { >>>>> + "error", "noreturn", "warning" >>>>> + }; >>>> >>>> >>>> Why whitelist noreturn? I would expect to want that to be consistent. >>> >>> I expect noreturn to be used on a primary whose definition >>> is provided but that's not meant to be used the way the API >>> is otherwise expected to be. As in: >>> >>> template <class T> >>> T [[noreturn]] foo () { throw "not implemented"; } >>> >>> template <> int foo<int>(); // implemented elsewhere >> >> Marking that template as noreturn seems pointless, and possibly harmful; >> the deprecated, warning, or error attributes would be better for this >> situation. > > I meant either: > > template <class T> > T __attribute__ ((noreturn)) foo () { throw "not implemented"; } > > template <> int foo<int>(); // implemented elsewhere > > or (sigh) > > template <class T> > [[noreturn]] T foo () { throw "not implemented"; } > > template <> int foo<int>(); // implemented elsewhere > > It lets code like this > > int bar () > { > return foo<char>(); > } > > be diagnosed because it's likely a bug (as Clang does with > -Wunreachable-code). It doesn't stop code like the following > from compiling (which is good) but it instead lets them throw > at runtime which is what foo's author wants. > > void bar () > { > foo<char>(); > } > > It's the same as having an "unimplemented" base virtual function > throw an exception when it's called rather than making it pure > and having calls to it abort. Declaring the base virtual function > noreturn is useful for the same reason (and also diagnosed by > Clang). I should remember to add the same warning in GCC 9. Yes, I understood the patterns you had in mind, but I disagree with them. My point about harmful is that declaring a function noreturn because it's unimplemented could be a problem for when the function is later implemented, and callers were optimized inappropriately. This seems like a rather roundabout way to get a warning about calling an unimplemented function, and not worth overriding the normal behavior. > I actually had some misgivings about both warning and deprecated > for the white-listing, but not for noreturn. My (only mild) > concern is that both warning and deprecated functions can and > likely will in some cases still be called, and so using them to > suppress the warning runs the risk that their calls might be > wrong and no one will notice. Warning cannot be suppressed > so it seems unlikely to be ignored, but deprecated can be. > So I wonder if the white-listing for deprecated should be > conditional on -Wdeprecated being enabled. > >>> - /* Merge the type qualifiers. */ >>> - if (TREE_READONLY (newdecl)) >>> - TREE_READONLY (olddecl) = 1; >>> if (TREE_THIS_VOLATILE (newdecl)) >>> TREE_THIS_VOLATILE (olddecl) = 1; >>> - if (TREE_NOTHROW (newdecl)) >>> - TREE_NOTHROW (olddecl) = 1; >>> + >>> + if (merge_attr) >>> + { >>> + /* Merge the type qualifiers. */ >>> + if (TREE_READONLY (newdecl)) >>> + TREE_READONLY (olddecl) = 1; >>> + } >>> + else >>> + { >>> + /* Set the bits that correspond to the const function >>> attributes. */ >>> + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); >>> + } >> >> >> Let's limit the const/volatile handling here to non-functions, and >> handle the const/noreturn attributes for functions in the later hunk >> along with nothrow/malloc/pure. > > > I had to keep the TREE_HAS_VOLATILE handling as is since it > applies to functions too (has side-effects). Otherwise the > attr-nothrow-2.C test fails. When I mentioned "noreturn" above I was referring to TREE_HAS_VOLATILE; sorry I wasn't clear. For functions it should be handled along with nothrow/readonly/malloc/pure. Jason
On 02/12/2018 09:30 AM, Jason Merrill wrote: > On Fri, Feb 9, 2018 at 6:57 PM, Martin Sebor <msebor@gmail.com> wrote: >> On 02/09/2018 12:52 PM, Jason Merrill wrote: >>> On 02/08/2018 04:52 PM, Martin Sebor wrote: >>>> >>>> I took me a while to find DECL_TEMPLATE_RESULT. Hopefully >>>> that's the right way to get the primary from a TEMPLATE_DECL. >>> >>> Yes. >>> >>>>>> Attached is an updated patch. It hasn't gone through full >>>>>> testing yet but please let me know if you'd like me to make >>>>>> some changes. >>>>> >>>>> >>>>>> + const char* const whitelist[] = { >>>>>> + "error", "noreturn", "warning" >>>>>> + }; >>>>> >>>>> >>>>> Why whitelist noreturn? I would expect to want that to be consistent. >>>> >>>> I expect noreturn to be used on a primary whose definition >>>> is provided but that's not meant to be used the way the API >>>> is otherwise expected to be. As in: >>>> >>>> template <class T> >>>> T [[noreturn]] foo () { throw "not implemented"; } >>>> >>>> template <> int foo<int>(); // implemented elsewhere >>> >>> Marking that template as noreturn seems pointless, and possibly harmful; >>> the deprecated, warning, or error attributes would be better for this >>> situation. >> >> I meant either: >> >> template <class T> >> T __attribute__ ((noreturn)) foo () { throw "not implemented"; } >> >> template <> int foo<int>(); // implemented elsewhere >> >> or (sigh) >> >> template <class T> >> [[noreturn]] T foo () { throw "not implemented"; } >> >> template <> int foo<int>(); // implemented elsewhere >> >> It lets code like this >> >> int bar () >> { >> return foo<char>(); >> } >> >> be diagnosed because it's likely a bug (as Clang does with >> -Wunreachable-code). It doesn't stop code like the following >> from compiling (which is good) but it instead lets them throw >> at runtime which is what foo's author wants. >> >> void bar () >> { >> foo<char>(); >> } >> >> It's the same as having an "unimplemented" base virtual function >> throw an exception when it's called rather than making it pure >> and having calls to it abort. Declaring the base virtual function >> noreturn is useful for the same reason (and also diagnosed by >> Clang). I should remember to add the same warning in GCC 9. > > Yes, I understood the patterns you had in mind, but I disagree with > them. My point about harmful is that declaring a function noreturn > because it's unimplemented could be a problem for when the function is > later implemented, and callers were optimized inappropriately. This > seems like a rather roundabout way to get a warning about calling an > unimplemented function, and not worth overriding the normal behavior. Removing noreturn from the whitelist means having to prevent the attribute from causing conflicts with the attributes on the blacklist. E.g., in this: template <class T> [[malloc]] void* allocate (int); template <> [[noreturn]] void* allocate<void> (int); -Wmissing-attributes would warn for the missing malloc but -Wattributes will warn once malloc is added. Ditto for all other attributes noreturn is considered to conflict with such as alloc_size and warn_unused_result. I anticipate the warning code to ultimately end up in the middle-end so it can handle Joseph's case as well, and so it can also be integrated with the attribute conflict machinery. It also needs to be in the middle-end to become usable by -Wsuggest-attribute. But I wasn't thinking of making any of these bigger changes until GCC 9. Do you want me to integrate it with the conflict stuff now? Martin >> I actually had some misgivings about both warning and deprecated >> for the white-listing, but not for noreturn. My (only mild) >> concern is that both warning and deprecated functions can and >> likely will in some cases still be called, and so using them to >> suppress the warning runs the risk that their calls might be >> wrong and no one will notice. Warning cannot be suppressed >> so it seems unlikely to be ignored, but deprecated can be. >> So I wonder if the white-listing for deprecated should be >> conditional on -Wdeprecated being enabled. >> >>>> - /* Merge the type qualifiers. */ >>>> - if (TREE_READONLY (newdecl)) >>>> - TREE_READONLY (olddecl) = 1; >>>> if (TREE_THIS_VOLATILE (newdecl)) >>>> TREE_THIS_VOLATILE (olddecl) = 1; >>>> - if (TREE_NOTHROW (newdecl)) >>>> - TREE_NOTHROW (olddecl) = 1; >>>> + >>>> + if (merge_attr) >>>> + { >>>> + /* Merge the type qualifiers. */ >>>> + if (TREE_READONLY (newdecl)) >>>> + TREE_READONLY (olddecl) = 1; >>>> + } >>>> + else >>>> + { >>>> + /* Set the bits that correspond to the const function >>>> attributes. */ >>>> + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); >>>> + } >>> >>> >>> Let's limit the const/volatile handling here to non-functions, and >>> handle the const/noreturn attributes for functions in the later hunk >>> along with nothrow/malloc/pure. >> >> >> I had to keep the TREE_HAS_VOLATILE handling as is since it >> applies to functions too (has side-effects). Otherwise the >> attr-nothrow-2.C test fails. > > When I mentioned "noreturn" above I was referring to > TREE_HAS_VOLATILE; sorry I wasn't clear. For functions it should be > handled along with nothrow/readonly/malloc/pure. > > Jason >
On Mon, Feb 12, 2018 at 11:59 AM, Martin Sebor <msebor@gmail.com> wrote: > On 02/12/2018 09:30 AM, Jason Merrill wrote: >> >> On Fri, Feb 9, 2018 at 6:57 PM, Martin Sebor <msebor@gmail.com> wrote: >>> >>> On 02/09/2018 12:52 PM, Jason Merrill wrote: >>>> >>>> On 02/08/2018 04:52 PM, Martin Sebor wrote: >>>>> >>>>> >>>>> I took me a while to find DECL_TEMPLATE_RESULT. Hopefully >>>>> that's the right way to get the primary from a TEMPLATE_DECL. >>>> >>>> >>>> Yes. >>>> >>>>>>> Attached is an updated patch. It hasn't gone through full >>>>>>> testing yet but please let me know if you'd like me to make >>>>>>> some changes. >>>>>> >>>>>> >>>>>> >>>>>>> + const char* const whitelist[] = { >>>>>>> + "error", "noreturn", "warning" >>>>>>> + }; >>>>>> >>>>>> >>>>>> >>>>>> Why whitelist noreturn? I would expect to want that to be consistent. >>>>> >>>>> >>>>> I expect noreturn to be used on a primary whose definition >>>>> is provided but that's not meant to be used the way the API >>>>> is otherwise expected to be. As in: >>>>> >>>>> template <class T> >>>>> T [[noreturn]] foo () { throw "not implemented"; } >>>>> >>>>> template <> int foo<int>(); // implemented elsewhere >>>> >>>> >>>> Marking that template as noreturn seems pointless, and possibly harmful; >>>> the deprecated, warning, or error attributes would be better for this >>>> situation. >>> >>> >>> I meant either: >>> >>> template <class T> >>> T __attribute__ ((noreturn)) foo () { throw "not implemented"; } >>> >>> template <> int foo<int>(); // implemented elsewhere >>> >>> or (sigh) >>> >>> template <class T> >>> [[noreturn]] T foo () { throw "not implemented"; } >>> >>> template <> int foo<int>(); // implemented elsewhere >>> >>> It lets code like this >>> >>> int bar () >>> { >>> return foo<char>(); >>> } >>> >>> be diagnosed because it's likely a bug (as Clang does with >>> -Wunreachable-code). It doesn't stop code like the following >>> from compiling (which is good) but it instead lets them throw >>> at runtime which is what foo's author wants. >>> >>> void bar () >>> { >>> foo<char>(); >>> } >>> >>> It's the same as having an "unimplemented" base virtual function >>> throw an exception when it's called rather than making it pure >>> and having calls to it abort. Declaring the base virtual function >>> noreturn is useful for the same reason (and also diagnosed by >>> Clang). I should remember to add the same warning in GCC 9. >> >> >> Yes, I understood the patterns you had in mind, but I disagree with >> them. My point about harmful is that declaring a function noreturn >> because it's unimplemented could be a problem for when the function is >> later implemented, and callers were optimized inappropriately. This >> seems like a rather roundabout way to get a warning about calling an >> unimplemented function, and not worth overriding the normal behavior. > > > Removing noreturn from the whitelist means having to prevent > the attribute from causing conflicts with the attributes on > the blacklist. E.g., in this: > > template <class T> [[malloc]] void* allocate (int); > > template <> [[noreturn]] void* allocate<void> (int); > > -Wmissing-attributes would warn for the missing malloc but > -Wattributes will warn once malloc is added. Ditto for all > other attributes noreturn is considered to conflict with such > as alloc_size and warn_unused_result. This example seems rather unlikely, and the solution is to remove [[noreturn]]. I don't think this is worth worrying about for GCC 8. > I anticipate the warning code to ultimately end up in > the middle-end so it can handle Joseph's case as well, and > so it can also be integrated with the attribute conflict > machinery. It also needs to be in the middle-end to become > usable by -Wsuggest-attribute. But I wasn't thinking of > making any of these bigger changes until GCC 9. > Do you want me to integrate it with the conflict stuff now? No, leaving it for GCC 9 makes sense to me. Jason >>> I actually had some misgivings about both warning and deprecated >>> for the white-listing, but not for noreturn. My (only mild) >>> concern is that both warning and deprecated functions can and >>> likely will in some cases still be called, and so using them to >>> suppress the warning runs the risk that their calls might be >>> wrong and no one will notice. Warning cannot be suppressed >>> so it seems unlikely to be ignored, but deprecated can be. >>> So I wonder if the white-listing for deprecated should be >>> conditional on -Wdeprecated being enabled. >>> >>>>> - /* Merge the type qualifiers. */ >>>>> - if (TREE_READONLY (newdecl)) >>>>> - TREE_READONLY (olddecl) = 1; >>>>> if (TREE_THIS_VOLATILE (newdecl)) >>>>> TREE_THIS_VOLATILE (olddecl) = 1; >>>>> - if (TREE_NOTHROW (newdecl)) >>>>> - TREE_NOTHROW (olddecl) = 1; >>>>> + >>>>> + if (merge_attr) >>>>> + { >>>>> + /* Merge the type qualifiers. */ >>>>> + if (TREE_READONLY (newdecl)) >>>>> + TREE_READONLY (olddecl) = 1; >>>>> + } >>>>> + else >>>>> + { >>>>> + /* Set the bits that correspond to the const function >>>>> attributes. */ >>>>> + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); >>>>> + } >>>> >>>> >>>> >>>> Let's limit the const/volatile handling here to non-functions, and >>>> handle the const/noreturn attributes for functions in the later hunk >>>> along with nothrow/malloc/pure. >>> >>> >>> >>> I had to keep the TREE_HAS_VOLATILE handling as is since it >>> applies to functions too (has side-effects). Otherwise the >>> attr-nothrow-2.C test fails. >> >> >> When I mentioned "noreturn" above I was referring to >> TREE_HAS_VOLATILE; sorry I wasn't clear. For functions it should be >> handled along with nothrow/readonly/malloc/pure. >> >> Jason >> >
On 02/12/2018 10:11 AM, Jason Merrill wrote: > On Mon, Feb 12, 2018 at 11:59 AM, Martin Sebor <msebor@gmail.com> wrote: >> On 02/12/2018 09:30 AM, Jason Merrill wrote: >>> >>> On Fri, Feb 9, 2018 at 6:57 PM, Martin Sebor <msebor@gmail.com> wrote: >>>> >>>> On 02/09/2018 12:52 PM, Jason Merrill wrote: >>>>> >>>>> On 02/08/2018 04:52 PM, Martin Sebor wrote: >>>>>> >>>>>> >>>>>> I took me a while to find DECL_TEMPLATE_RESULT. Hopefully >>>>>> that's the right way to get the primary from a TEMPLATE_DECL. >>>>> >>>>> >>>>> Yes. >>>>> >>>>>>>> Attached is an updated patch. It hasn't gone through full >>>>>>>> testing yet but please let me know if you'd like me to make >>>>>>>> some changes. >>>>>>> >>>>>>> >>>>>>> >>>>>>>> + const char* const whitelist[] = { >>>>>>>> + "error", "noreturn", "warning" >>>>>>>> + }; >>>>>>> >>>>>>> >>>>>>> >>>>>>> Why whitelist noreturn? I would expect to want that to be consistent. >>>>>> >>>>>> >>>>>> I expect noreturn to be used on a primary whose definition >>>>>> is provided but that's not meant to be used the way the API >>>>>> is otherwise expected to be. As in: >>>>>> >>>>>> template <class T> >>>>>> T [[noreturn]] foo () { throw "not implemented"; } >>>>>> >>>>>> template <> int foo<int>(); // implemented elsewhere >>>>> >>>>> >>>>> Marking that template as noreturn seems pointless, and possibly harmful; >>>>> the deprecated, warning, or error attributes would be better for this >>>>> situation. >>>> >>>> >>>> I meant either: >>>> >>>> template <class T> >>>> T __attribute__ ((noreturn)) foo () { throw "not implemented"; } >>>> >>>> template <> int foo<int>(); // implemented elsewhere >>>> >>>> or (sigh) >>>> >>>> template <class T> >>>> [[noreturn]] T foo () { throw "not implemented"; } >>>> >>>> template <> int foo<int>(); // implemented elsewhere >>>> >>>> It lets code like this >>>> >>>> int bar () >>>> { >>>> return foo<char>(); >>>> } >>>> >>>> be diagnosed because it's likely a bug (as Clang does with >>>> -Wunreachable-code). It doesn't stop code like the following >>>> from compiling (which is good) but it instead lets them throw >>>> at runtime which is what foo's author wants. >>>> >>>> void bar () >>>> { >>>> foo<char>(); >>>> } >>>> >>>> It's the same as having an "unimplemented" base virtual function >>>> throw an exception when it's called rather than making it pure >>>> and having calls to it abort. Declaring the base virtual function >>>> noreturn is useful for the same reason (and also diagnosed by >>>> Clang). I should remember to add the same warning in GCC 9. >>> >>> >>> Yes, I understood the patterns you had in mind, but I disagree with >>> them. My point about harmful is that declaring a function noreturn >>> because it's unimplemented could be a problem for when the function is >>> later implemented, and callers were optimized inappropriately. This >>> seems like a rather roundabout way to get a warning about calling an >>> unimplemented function, and not worth overriding the normal behavior. >> >> >> Removing noreturn from the whitelist means having to prevent >> the attribute from causing conflicts with the attributes on >> the blacklist. E.g., in this: >> >> template <class T> [[malloc]] void* allocate (int); >> >> template <> [[noreturn]] void* allocate<void> (int); >> >> -Wmissing-attributes would warn for the missing malloc but >> -Wattributes will warn once malloc is added. Ditto for all >> other attributes noreturn is considered to conflict with such >> as alloc_size and warn_unused_result. > > This example seems rather unlikely, and the solution is to remove > [[noreturn]]. I don't think this is worth worrying about for GCC 8. Removing [[noreturn]] is not a solution because (as I said) -Wmissing-attributes will warn that the specialization is missing the malloc attribute. The only solutions to avoid both warnings are to either a) remove the malloc attribute (and all others on the blacklist) from the primary or b) add all of them to the specialization and remove the noreturn. I.e., have them match. That makes sense except when either the primary or the specialization in fact does not return. I really don't think it's helpful to try to force noreturn to match between the primary and its specializations. The use case you are concerned about (a noreturn function returning) is already diagnosed: warning: function declared ‘noreturn’ has a ‘return’ statement Martin
On Mon, Feb 12, 2018 at 6:39 PM, Martin Sebor <msebor@gmail.com> wrote: > I really don't think it's helpful to try to force noreturn > to match between the primary and its specializations. I continue to disagree. Jason
Hi, On Mon, 12 Feb 2018, Martin Sebor wrote: > >> Removing noreturn from the whitelist means having to prevent > >> the attribute from causing conflicts with the attributes on > >> the blacklist. E.g., in this: > >> > >> template <class T> [[malloc]] void* allocate (int); > >> > >> template <> [[noreturn]] void* allocate<void> (int); Marking a function having a return type as noreturn doesn't make sense. So a warning in this case is actually a good thing. And changing the return type to void (so that noreturn makes sense) makes it not a specialization anymore (or alternatively if the primary is also changed to void then malloc doesn't make sense anymore). Ciao, Michael.
On 02/13/2018 08:59 AM, Michael Matz wrote: > Hi, > > On Mon, 12 Feb 2018, Martin Sebor wrote: > >>>> Removing noreturn from the whitelist means having to prevent >>>> the attribute from causing conflicts with the attributes on >>>> the blacklist. E.g., in this: >>>> >>>> template <class T> [[malloc]] void* allocate (int); >>>> >>>> template <> [[noreturn]] void* allocate<void> (int); > > Marking a function having a return type as noreturn doesn't make sense. No, that's certainly not so in general(*). It makes sense when a function cannot meaningfully be implemented to honor its broader API contract (this applies to overloads, virtual functions, and also template specializations). In the example above, the author of the allocate template may wish to have it allocate and initialize an array of T which can only be implemented for non-void types so they define allocate like this: template <class T> [[gnu::malloc]] T* allocate (int n) { return new T[n]; } template <> [[noreturn]] void* allocate<void> (int) { throw "cannot allocate void arrays"; } Defining a specialization that differs from a primary or vice versa is a common idiom that I don't want to force users to abandon. As I already mentioned, this idiom has a parallel in object oriented code (as opposed to in generic code) where a "pure" virtual functions are declared to return a non-void type and defined to exit/throw/abort, or not defined at all (i.e., the same as abort). In these case, warning would on calls to such functions would be helpful the same that warning on noreturn functions > So a warning in this case is actually a good thing. A warning is only useful if it detects a bug or suggests a potential improvement. The bug Jason is concerned about having the warning detect involves changing the allocate specialization in the future to return a value without removing the noreturn atttribute. But that bug doesn't exist with the implementation above and is already diagnosed with the changed implementation, so there is no reason to warn for it now. No other use case/concern where a warning on the above might be useful has been brought up so, AFAICS, the only instances of the warning will be false positives. > And changing the > return type to void (so that noreturn makes sense) makes it not a > specialization anymore (or alternatively if the primary is also changed to > void then malloc doesn't make sense anymore). Exactly. The only way to suppress the warning is to either remove the attributes from the primary, or duplicate them on the specialization. Neither approach would be correct because neither would reflect the property of the declaration it's applied to. Martin [*] There are a number of practical examples that show where noreturn is useful with non-void return types. The common ones involve overloads of APIs with an expected signature that are not/cannot be implemented to return a value. See the cfe-dev discussion at http://lists.llvm.org/pipermail/cfe-dev/2011-May/014969.html for one such example. For another one see N4226 where applying noreturn to main was considered sufficiently useful to justify a C++ proposal for an enhancement. (AFAIK, the proposal hasn't yet been accepted.
On 02/13/2018 07:47 AM, Jason Merrill wrote: > On Mon, Feb 12, 2018 at 6:39 PM, Martin Sebor <msebor@gmail.com> wrote: >> I really don't think it's helpful to try to force noreturn >> to match between the primary and its specializations. > > I continue to disagree. Can you explain what use case you are concerned about that isn't already handled by the warning about noreturn function returning? For reference, the cases I consider important are: 1) "Unimplemented" primary template declared noreturn that throws or exits but whose specializations return a useful value and make use of attribute malloc (or one of the other blacklisted attributes). 2) The converse of (1). A primary that returns a useful malloc like value and some of whose specializations are not/cannot be meaningfully implemented and are declared noreturn. Defining template specializations that differ from the primary template in their implementation is idiomatic (analogous to defining overloads or overridden virtual functions). In any event, I am mainly interested in fixing the two bugs (one a P1 regression). If you consider changing the warning aspect of the patch a condition of accepting the fix please let me know. Removing the noreturn keyword from the whitelist is trivial. Martin
On Tue, Feb 13, 2018 at 1:00 PM, Martin Sebor <msebor@gmail.com> wrote: > On 02/13/2018 07:47 AM, Jason Merrill wrote: >> >> On Mon, Feb 12, 2018 at 6:39 PM, Martin Sebor <msebor@gmail.com> wrote: >>> >>> I really don't think it's helpful to try to force noreturn >>> to match between the primary and its specializations. >> >> I continue to disagree. > > Can you explain what use case you are concerned about that isn't > already handled by the warning about noreturn function returning? A specialization that forgot [[noreturn]] and therefore doesn't get the desired warning. > For reference, the cases I consider important are: > > 1) "Unimplemented" primary template declared noreturn that throws > or exits but whose specializations return a useful value and > make use of attribute malloc (or one of the other blacklisted > attributes). > > 2) The converse of (1). A primary that returns a useful malloc > like value and some of whose specializations are not/cannot > be meaningfully implemented and are declared noreturn. Right, but I still disagree with this use of noreturn, and therefore don't consider these cases important. > Defining template specializations that differ from the primary > template in their implementation is idiomatic (analogous to > defining overloads or overridden virtual functions). > In any event, I am mainly interested in fixing the two bugs > (one a P1 regression). If you consider changing the warning > aspect of the patch a condition of accepting the fix please let > me know. Removing the noreturn keyword from the whitelist is > trivial. Please do. Jason
On 02/13/2018 11:33 AM, Jason Merrill wrote: > On Tue, Feb 13, 2018 at 1:00 PM, Martin Sebor <msebor@gmail.com> wrote: >> On 02/13/2018 07:47 AM, Jason Merrill wrote: >>> >>> On Mon, Feb 12, 2018 at 6:39 PM, Martin Sebor <msebor@gmail.com> wrote: >>>> >>>> I really don't think it's helpful to try to force noreturn >>>> to match between the primary and its specializations. >>> >>> I continue to disagree. >> >> Can you explain what use case you are concerned about that isn't >> already handled by the warning about noreturn function returning? > > A specialization that forgot [[noreturn]] and therefore doesn't get > the desired warning. > >> For reference, the cases I consider important are: >> >> 1) "Unimplemented" primary template declared noreturn that throws >> or exits but whose specializations return a useful value and >> make use of attribute malloc (or one of the other blacklisted >> attributes). >> >> 2) The converse of (1). A primary that returns a useful malloc >> like value and some of whose specializations are not/cannot >> be meaningfully implemented and are declared noreturn. > > Right, but I still disagree with this use of noreturn, and therefore > don't consider these cases important. > >> Defining template specializations that differ from the primary >> template in their implementation is idiomatic (analogous to >> defining overloads or overridden virtual functions). > >> In any event, I am mainly interested in fixing the two bugs >> (one a P1 regression). If you consider changing the warning >> aspect of the patch a condition of accepting the fix please let >> me know. Removing the noreturn keyword from the whitelist is >> trivial. > > Please do. Attached is an updated patch with this change and with the TREE_HAS_VOLATILE bit split up between types and functions. I've also removed warn_unused_result from the blacklist (see below). Bootstrapped on x864_64, regression test is in progress. Martin While reviewing other related bugs I noticed 83502. This patch doesn't fix the first test case in the bug (attribute noinline vs always_inline). Somehow those are still copied from the primary to the specialization and can cause conflicts. It does fix the second test case but with the noreturn change it would issue a bogus -Wmissing-attributes warning for the explicit specialization below. Adding the warn_unused_result attribute to it would then make GCC complain about a conflict between the added attribute and noreturn, while removing it would lead to worse code. template <class T> int __attribute__ ((warn_unused_result)) f (T) { return 0; } template <> int __attribute__ ((noreturn)) f<int> (int) { throw 0; } void fi () { f (0); } PR c++/83871 - wrong code for attribute const and pure on distinct template specializations PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization gcc/ChangeLog: PR c++/83871 * gcc/doc/invoke.texi (-Wmissing-attributes): New option. gcc/c-family/ChangeLog: PR c++/83871 * c.opt (-Wmissing-attributes): New option. gcc/cp/ChangeLog: PR c++/83871 PR c++/83503 * cp-tree.h (warn_spec_missing_attributes): New function. ((check_explicit_specialization): Add an argument. Call the above function. * decl.c (duplicate_decls): Avoid applying primary function template's attributes to its explicit specializations. gcc/testsuite/ChangeLog: PR c++/83871 PR c++/83503 * g++.dg/ext/attr-const-pure.C: New test. * g++.dg/ext/attr-malloc.C: New test. * g++.dg/ext/attr-nonnull.C: New test. * g++.dg/ext/attr-nothrow.C: New test. * g++.dg/ext/attr-nothrow-2.C: New test. * g++.dg/ext/attr-returns-nonnull.C: New test. * g++.dg/Wmissing-attributes.C: New test. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 7fb386d..1d0682d 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -781,6 +781,11 @@ Wtemplates C++ ObjC++ Var(warn_templates) Warning Warn on primary template declaration. +Wmissing-attributes +C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn about declarations of entities that may be missing attributes +that related entities have been declared with it. + Wmissing-format-attribute C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format) ; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a6c75ae..6255914 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6469,7 +6469,8 @@ extern void end_specialization (void); extern void begin_explicit_instantiation (void); extern void end_explicit_instantiation (void); extern void check_unqualified_spec_or_inst (tree, location_t); -extern tree check_explicit_specialization (tree, tree, int, int); +extern tree check_explicit_specialization (tree, tree, int, int, + tree = NULL_TREE); extern int num_template_headers_for_class (tree); extern void check_template_variable (tree); extern tree make_auto (void); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 3ccea9e..1f012ef 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -1969,10 +1969,21 @@ next_arg:; DECL_ORIGINAL_TYPE (newdecl) = DECL_ORIGINAL_TYPE (olddecl); } - /* Copy all the DECL_... slots specified in the new decl - except for any that we copy here from the old type. */ - DECL_ATTRIBUTES (newdecl) - = (*targetm.merge_decl_attributes) (olddecl, newdecl); + /* True to merge attributes between the declarations, false to + set OLDDECL's attributes to those of NEWDECL (for template + explicit specializations that specify their own attributes + independent of those specified for the primary template). */ + const bool merge_attr = (TREE_CODE (newdecl) != FUNCTION_DECL + || !DECL_TEMPLATE_SPECIALIZATION (newdecl) + || DECL_TEMPLATE_SPECIALIZATION (olddecl)); + + /* Copy all the DECL_... slots specified in the new decl except for + any that we copy here from the old type. */ + if (merge_attr) + DECL_ATTRIBUTES (newdecl) + = (*targetm.merge_decl_attributes) (olddecl, newdecl); + else + DECL_ATTRIBUTES (olddecl) = DECL_ATTRIBUTES (newdecl); if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl)) { @@ -2099,9 +2110,10 @@ next_arg:; } } } - else - /* Merge the data types specified in the two decls. */ + else if (merge_attr) newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); + else + newtype = TREE_TYPE (newdecl); if (VAR_P (newdecl)) { @@ -2165,14 +2177,6 @@ next_arg:; && !(processing_template_decl && uses_template_parms (newdecl))) layout_decl (newdecl, 0); - /* Merge the type qualifiers. */ - if (TREE_READONLY (newdecl)) - TREE_READONLY (olddecl) = 1; - if (TREE_THIS_VOLATILE (newdecl)) - TREE_THIS_VOLATILE (olddecl) = 1; - if (TREE_NOTHROW (newdecl)) - TREE_NOTHROW (olddecl) = 1; - /* Merge deprecatedness. */ if (TREE_DEPRECATED (newdecl)) TREE_DEPRECATED (olddecl) = 1; @@ -2190,6 +2194,15 @@ next_arg:; DECL_FUNCTION_SPECIFIC_OPTIMIZATION (newdecl) = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (olddecl); } + else + { + /* Merge the const type qualifier. */ + if (TREE_READONLY (newdecl)) + TREE_READONLY (olddecl) = 1; + /* Merge the volatile type qualifier. */ + if (TREE_THIS_VOLATILE (newdecl)) + TREE_THIS_VOLATILE (olddecl) = 1; + } /* Merge the initialization information. */ if (DECL_INITIAL (newdecl) == NULL_TREE @@ -2210,13 +2223,28 @@ next_arg:; |= DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (olddecl); DECL_NO_LIMIT_STACK (newdecl) |= DECL_NO_LIMIT_STACK (olddecl); TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); - TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); - DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); - DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); - TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); - DECL_LOOPING_CONST_OR_PURE_P (newdecl) + DECL_LOOPING_CONST_OR_PURE_P (newdecl) |= DECL_LOOPING_CONST_OR_PURE_P (olddecl); + + /* Merge the function-has-side-effects bit. */ + if (TREE_THIS_VOLATILE (newdecl)) + TREE_THIS_VOLATILE (olddecl) = 1; + + if (merge_attr) + { + TREE_NOTHROW (olddecl) |= TREE_NOTHROW (newdecl); + TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); + DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); + DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); + } + else + { + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); + DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl); + } /* Keep the old RTL. */ COPY_DECL_RTL (olddecl, newdecl); } @@ -8910,7 +8938,8 @@ grokfndecl (tree ctype, template_count, 2 * funcdef_flag + 4 * (friendp != 0) + - 8 * concept_p); + 8 * concept_p, + *attrlist); if (decl == error_mark_node) return NULL_TREE; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index b58c60f..ffd1eaf 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. If not see all methods must be provided in header files; can't use a source file that contains only the method templates and "just win". */ +#include <string> #include "config.h" #include "system.h" #include "coretypes.h" @@ -1473,8 +1474,10 @@ is_specialization_of_friend (tree decl, tree friend_decl) /* Register the specialization SPEC as a specialization of TMPL with the indicated ARGS. IS_FRIEND indicates whether the specialization - is actually just a friend declaration. Returns SPEC, or an - equivalent prior declaration, if available. + is actually just a friend declaration. ATTRLIST is the list of + attributes that the specialization is declared with or NULL when + it isn't. Returns SPEC, or an equivalent prior declaration, if + available. We also store instantiations of field packs in the hash table, even though they are not themselves templates, to make lookup easier. */ @@ -2610,6 +2613,110 @@ check_unqualified_spec_or_inst (tree t, location_t loc) } } +/* Warn for a template specialization SPEC that is missing some of a set + of function or type attributes that the template TEMPL is declared with. + ATTRLIST is a list of additional attributes that SPEC should be taken + to ultimately be declared with. */ + +static void +warn_spec_missing_attributes (tree tmpl, tree spec, tree attrlist) +{ + if (DECL_FUNCTION_TEMPLATE_P (tmpl)) + tmpl = DECL_TEMPLATE_RESULT (tmpl); + + if (TREE_CODE (tmpl) != FUNCTION_DECL) + return; + + /* Avoid warning if either declaration or its type is deprecated. */ + if (TREE_DEPRECATED (tmpl) + || TREE_DEPRECATED (spec)) + return; + + tree tmpl_type = TREE_TYPE (tmpl); + tree spec_type = TREE_TYPE (spec); + + if (TREE_DEPRECATED (tmpl_type) + || TREE_DEPRECATED (spec_type) + || TREE_DEPRECATED (TREE_TYPE (tmpl_type)) + || TREE_DEPRECATED (TREE_TYPE (spec_type))) + return; + + tree tmpl_attrs[] = { DECL_ATTRIBUTES (tmpl), TYPE_ATTRIBUTES (tmpl_type) }; + tree spec_attrs[] = { DECL_ATTRIBUTES (spec), TYPE_ATTRIBUTES (spec_type) }; + + if (!spec_attrs[0]) + spec_attrs[0] = attrlist; + else if (!spec_attrs[1]) + spec_attrs[1] = attrlist; + + /* Avoid warning if the primary has no attributes. */ + if (!tmpl_attrs[0] && !tmpl_attrs[1]) + return; + + /* Avoid warning if either declaration contains an attribute on + the white list below. */ + const char* const whitelist[] = { + "error", "warning" + }; + + for (unsigned i = 0; i != 2; ++i) + for (unsigned j = 0; j != sizeof whitelist / sizeof *whitelist; ++j) + if (lookup_attribute (whitelist[j], tmpl_attrs[i]) + || lookup_attribute (whitelist[j], spec_attrs[i])) + return; + + /* Avoid warning if the difference between the primary and + the specialization is not in one of the attributes below. */ + const char* const blacklist[] = { + "alloc_align", "alloc_size", "assume_aligned", "format", + "format_arg", "malloc", "nonnull" + }; + + /* Put together a list of the black listed attributes that the primary + template is declared with that the specialization is not, in case + it's not apparent from the most recent declaration of the primary. */ + unsigned nattrs = 0; + std::string str; + + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) + { + for (unsigned j = 0; j != 2; ++j) + { + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) + continue; + + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) + { + if (lookup_attribute (blacklist[i], spec_attrs[k])) + break; + + if (str.size ()) + str += ", "; + str += "%<"; + str += blacklist[i]; + str += "%>"; + ++nattrs; + } + } + } + + if (!nattrs) + return; + + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attributes, + "explicit specialization %q#D may be missing attributes", + spec)) + { + if (nattrs > 1) + str = G_("missing primary template attributes ") + str; + else + str = G_("missing primary template attribute ") + str; + + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); + } + +} + /* Check to see if the function just declared, as indicated in DECLARATOR, and in DECL, is a specialization of a function template. We may also discover that the declaration is an explicit @@ -2651,7 +2758,8 @@ tree check_explicit_specialization (tree declarator, tree decl, int template_count, - int flags) + int flags, + tree attrlist) { int have_def = flags & 2; int is_friend = flags & 4; @@ -3108,8 +3216,13 @@ check_explicit_specialization (tree declarator, it again. Partial specializations will be registered in process_partial_specialization. */ if (!processing_template_decl) - decl = register_specialization (decl, gen_tmpl, targs, - is_friend, 0); + { + warn_spec_missing_attributes (gen_tmpl, decl, attrlist); + + decl = register_specialization (decl, gen_tmpl, targs, + is_friend, 0); + } + /* A 'structor should already have clones. */ gcc_assert (decl == error_mark_node diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 9db9d08..56d1843 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -295,7 +295,7 @@ Objective-C and Objective-C++ Dialects}. -Winvalid-pch -Wlarger-than=@var{len} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol --Wmisleading-indentation -Wmissing-braces @gol +-Wmisleading-indentation -Wmissing-attributes -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-include-dirs @gol -Wno-multichar -Wmultistatement-macros -Wnonnull -Wnonnull-compare @gol -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol @@ -3925,6 +3925,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. -Wmemset-elt-size @gol -Wmemset-transposed-args @gol -Wmisleading-indentation @r{(only for C/C++)} @gol +-Wmissing-attributes @gol -Wmissing-braces @r{(only for C/ObjC)} @gol -Wmultistatement-macros @gol -Wnarrowing @r{(only for C++)} @gol @@ -4588,6 +4589,36 @@ about the layout of the file that the directive references. This warning is enabled by @option{-Wall} in C and C++. +@item -Wmissing-attributes +@opindex Wmissing-attributes +@opindex Wno-missing-attributes +Warn when a declaration of a function is missing one or more attributes +that a related function is declared with and whose absence may adversely +affect the correctness or efficiency of generated code. For example, in +C++, the warning is issued when an explicit specialization of a primary +template declared with attribute @code{alloc_align}, @code{alloc_size}, +@code{assume_aligned}, @code{format}, @code{format_arg}, @code{malloc}, +or @code{nonnull} is declared without it. Attributes @code{deprecated}, +@code{error}, and @code{warning} suppress the warning. +(@pxref{Function Attributes}). + +@option{-Wmissing-attributes} is enabled by @option{-Wall}. + +For example, since the declaration of the primary function template +below makes use of both attribute @code{malloc} and @code{alloc_size} +the declaration of the explicit specialization of the template is +diagnosed because it is missing one of the attributes. + +@smallexample +template <class T> +T* __attribute__ ((malloc, alloc_size (1))) +allocate (size_t); + +template <> +void* __attribute__ ((malloc)) // missing alloc_size +allocate<void> (size_t); +@end smallexample + @item -Wmissing-braces @opindex Wmissing-braces @opindex Wno-missing-braces diff --git a/gcc/testsuite/g++.dg/Wmissing-attributes.C b/gcc/testsuite/g++.dg/Wmissing-attributes.C new file mode 100644 index 0000000..f4ebce1 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wmissing-attributes.C @@ -0,0 +1,102 @@ +// PR c++/83871 - wrong code for attribute const and pure on distinct +// template specializations +// Test to verify that a declaration of an explicit specialization with +// no attributes is diagnosed when the primary template is declared with +// one or more attributes. The warning helps highlight a change in GCC +// 8 from previous versions that copied the attributes from the primary +// to the specialization. It also helps point out simply forgetting to +// declare the specialization with an attribute. +// { dg-do compile } +// { dg-options "-Wmissing-attributes" } + +#define ATTR(list) __attribute__ (list) + + +// Verify that a primary without attributes doesn't cause warnings. +template <class T> void fnoattr (); + +template <> void fnoattr<void>(); +template <> void ATTR ((cold)) fnoattr<int>(); +template <> void ATTR ((hot)) fnoattr<double>(); + +// Verify that a noreturn primary also doesn't cause warnings. +template <class T> int ATTR ((noreturn)) fnoreturn (); + +template <> int fnoreturn<void>(); +template <> int ATTR ((cold)) fnoreturn<int>(); +template <> int ATTR ((hot)) fnoreturn<double>(); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_all (int); // { dg-message "missing primary template attributes \(.malloc., .alloc_size.|.alloc_size., .malloc.\)" } + +template <> +void* +missing_all<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +// Verify that specifying the same attributes in whatever order +// doesn't trigger the warning, even when other attributes are +// added. +template <> +void* +ATTR ((alloc_size (1), malloc)) +missing_all<char>(int); + +template <> +void* +ATTR ((alloc_size (1))) ATTR ((malloc)) ATTR ((returns_nonnull)) +missing_all<char>(int); // T = char, same as above + +template <> +void* +ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc)) +missing_all<char>(int); // T = char, same as above + +// Verify that the following attributes suppress the warning. +template <> void* ATTR ((error (""))) missing_all<short>(int); +template <> void* ATTR ((deprecated)) missing_all<int>(int); +template <> void* ATTR ((warning (""))) missing_all<double>(int); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_malloc (int); // { dg-message "missing primary template attribute .malloc." } + +template <> +void* +ATTR ((alloc_size (1))) +missing_malloc<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((malloc, alloc_size (1))) missing_malloc<short>(int); +template <> void* ATTR ((deprecated)) missing_malloc<int>(int); +template <> void* ATTR ((error (""))) missing_malloc<long>(int); +template <> void* ATTR ((warning (""))) missing_malloc<double>(int); + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_alloc_size (int, int); // { dg-message "missing primary template attribute .alloc_size." } + +template <> +void* +ATTR ((malloc)) +missing_alloc_size<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + + +template <class T> +void* +ATTR ((nonnull (1))) +missing_nonnull (void*); // { dg-message "missing primary template attribute .nonnull." } + +template <> +void* +ATTR ((malloc)) +missing_nonnull<char>(void*); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((nonnull (1))) missing_nonnull<short>(void*); +template <> void* ATTR ((deprecated)) missing_nonnull<int>(void*); +template <> void* ATTR ((error (""))) missing_nonnull<long>(void*); +template <> void* ATTR ((warning (""))) missing_nonnull<double>(void*); diff --git a/gcc/testsuite/g++.dg/ext/attr-const-pure.C b/gcc/testsuite/g++.dg/ext/attr-const-pure.C new file mode 100644 index 0000000..f7c6f3b --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const-pure.C @@ -0,0 +1,144 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +int global; + +template <class T> +int __attribute__ ((pure)) +f (T); + +template <> +int __attribute__ ((const)) f<int> (int); // { dg-bogus "ignoring attribute .const." } + +void f_pure_primary_elim (); +void f_pure_primary_keep (); +void f_const_spec_elim (); + +void call_pure_primary_elim (double x) +{ + // Only the first call to f(x) must be emitted, the second one + // is expected to be eliminated because the primary template + // is pure. + int i0 = f (x); + int i1 = f (x); + if (i0 != i1) + f_pure_primary_elim (); +} + +void call_pure_primary_keep (const char *s) +{ + // Both calls to f(x) must be emitted because the primary is + // pure and may read global. + int i0 = f (s); + global = 123; + int i1 = f (s); + if (i0 != i1) + f_pure_primary_keep (); +} + +void call_const_spec_elim (int i) +{ + // Only the first call to f(x) must be emitted, the second + // one is expected to be eliminated again, this time because + // unlike the pure primary, the specialization is const. + int i0 = f (i); + global = 123; + int i1 = f (i); + if (i0 != i1) + f_const_spec_elim (); +} + +template <class T> +int __attribute__ ((const)) +g (T); + +template <> +int __attribute__ ((pure)) g<int> (int); // { dg-bogus "ignoring attribute .const." } + +template <class T> +int __attribute__ ((const)) +h (T); + +template <class T> +int __attribute__ ((pure)) +h (const T*); + +template <> +int h<int> (int); + +template <> +int h<int*> (int*); + +extern void h_const_primary_elim (); +extern void h_pure_cstptr_elim (); +extern void h_cstptr_keep (); +extern void h_int_keep (); +extern void h_intptr_keep (); + +void call_const_primary_elim (double x) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. + int i0 = h (x); + int i1 = h (x); + + if (i0 != i1) // must be folded into false + h_const_primary_elim (); // must be eliminated +} + +void call_pure_cstptr_elim (const void *p) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. This verifies that h<const + // void*>*() is treated as const in this context. + int i0 = h (p); + int i1 = h (p); + + if (i0 != i1) // must be folded into false + h_pure_cstptr_elim (); // must be eliminated +} + +void call_cstptr_keep (const void *p) +{ + // Because of the store to the global, both calls to h(p) must + // be emitted. This verifies that h<const void*>*() is not + // treated as const. + int i0 = h (p); + global = 123; + int i1 = h (p); + + if (i0 != i1) // must not be folded + h_cstptr_keep (); // must be emitted +} + +void call_int_keep (int i) +{ + // Both calls to h(i) must be emitted. + int i0 = h (i); + int i1 = h (i); + + if (i0 != i1) // must not be folded + h_int_keep (); // must be emitted +} + +void call_intptr_keep (int *ip) +{ + // Both calls to h(ip) must be emitted. + int i0 = h (ip); + int i1 = h (ip); + + if (i0 != i1) // must not be folded + h_intptr_keep (); // must be emitted +} + +// { dg-final { scan-tree-dump-not "f_pure_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "f_const_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "f_const_spec_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "h_pure_cstptr_elim" "optimized" } } + +// { dg-final { scan-tree-dump-times "f_pure_primary_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_cstptr_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_int_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_intptr_keep" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc.C b/gcc/testsuite/g++.dg/ext/attr-malloc.C new file mode 100644 index 0000000..ce76abe --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc.C @@ -0,0 +1,45 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that an explicit template specifialization does not +// "inherit" attribute malloc from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((malloc)) +f (unsigned); + +template <> +void* +f<int>(unsigned); // { dg-warning "may be missing attributes" } + +static char a[8]; + +void f_void_malloc (); +void f_int_not_malloc (); + +void fv (void) +{ + void *p = f<void>(1); + if (!p) + return; + + if (p == a) // must be false + f_void_malloc (); // should be eliminated +} + + +void fi (void) +{ + void *p = f<int>(1); + if (!p) + return; + + if (p == a) // can be true + f_int_not_malloc (); // must not be eliminated +} + +// Verify that the call to f_void_not_malloc() is eliminated but +// the call to f_int_not_malloc() is retained. +// { dg-final { scan-tree-dump-not "f_void_malloc" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_not_malloc" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-nonnull.C new file mode 100644 index 0000000..57d2cb0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nonnull.C @@ -0,0 +1,31 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nonnull (1))) +f (T*, T*, T*); + +template <> +void +f<int>(int*, int*, int*); // { dg-warning "may be missing attributes" } + +template <> +void __attribute__ ((nonnull (3))) +f<float>(float*, float*, float*); + + +void test_nonnull (void) +{ + f<void>(0, 0, 0); // { dg-warning "null argument where non-null required \\\(argument 1\\\)" } + + f<int>(0, 0, 0); // { dg-bogus "null argument" } + + f<float>(0, 0, 0); + // { dg-bogus "null argument where non-null required \\\(argument 1\\\)" "" { target *-*-* } .-1 } + // { dg-warning "null argument where non-null required \\\(argument 3\\\)" "" { target *-*-* } .-2 } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C new file mode 100644 index 0000000..4061f82 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C @@ -0,0 +1,36 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + { dg-do compile } + { dg-options "-O -fdump-tree-eh" } */ + +void __attribute__ ((nothrow)) f (); + +void ff () throw () +{ + // No exception handling necessary around the call to f(). + f (); +} + +void __attribute__ ((nothrow)) g (); +void g (); + +void gg () throw () +{ + // No exception handling necessary around the call to g(). + g (); +} + +int __attribute__ ((nothrow)) h (); +int __attribute__ ((noreturn)) h (); +int h (); + +int hh () throw () +{ + // No exception handling necessary around the call to h(). + // No -Wreturn-value should be emitted because h is noreturn. + h (); +} + +// Verify that no exception handling code was emitted. +// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } } +// { dg-final { scan-tree-dump-not "resx" "eh" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow.C b/gcc/testsuite/g++.dg/ext/attr-nothrow.C new file mode 100644 index 0000000..ae6b674 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow.C @@ -0,0 +1,47 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute nothrow from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nothrow)) +f (); + +template <> +void +f<int>(); + +void f_void_nothrow (); +void f_int_maythrow (); + +void fv (void) +{ + try + { + f<void>(); + } + catch (...) // cannot be be reached + { + f_void_nothrow (); // should be eliminated + } +} + + +void fi (void) +{ + try + { + f<int>(); + } + catch (...) // may be reached + { + f_int_maythrow (); // must not be eliminated + } +} + +// Verify that the call to f_void_nothrow() is eliminated but +// the call to f_int_maythrow() is retained. +// { dg-final { scan-tree-dump-not "f_void_nothrow" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_maythrow" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C new file mode 100644 index 0000000..f75f32e --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C @@ -0,0 +1,42 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((returns_nonnull)) +g (); + +template <> +void* +g<int>(); + +extern void g_void_returns_nonnull (); +extern void g_int_may_return_null (); + +void test_returns_nonnull () +{ + void *p = g<void>(); + if (!p) + g_void_returns_nonnull (); + + (void)&p; +} + +void test_may_return_null () +{ + void *p = g<int>(); + if (!p) + g_int_may_return_null (); + + (void)&p; +} + + +// Verify that the call to g_void_returns_nonnull() is eliminated but +// the call to g_int_may_return_null() is retained. +// { dg-final { scan-tree-dump-not "g_void_returns_nonnull" "optimized" } } +// { dg-final { scan-tree-dump-times "g_int_may_return_null" 1 "optimized" } }
On 02/21/2018 06:03 PM, Martin Sebor wrote: > On 02/13/2018 11:33 AM, Jason Merrill wrote: >> On Tue, Feb 13, 2018 at 1:00 PM, Martin Sebor <msebor@gmail.com> wrote: >>> On 02/13/2018 07:47 AM, Jason Merrill wrote: >>>> >>>> On Mon, Feb 12, 2018 at 6:39 PM, Martin Sebor <msebor@gmail.com> wrote: >>>>> >>>>> I really don't think it's helpful to try to force noreturn >>>>> to match between the primary and its specializations. >>>> >>>> I continue to disagree. >>> >>> Can you explain what use case you are concerned about that isn't >>> already handled by the warning about noreturn function returning? >> >> A specialization that forgot [[noreturn]] and therefore doesn't get >> the desired warning. >> >>> For reference, the cases I consider important are: >>> >>> 1) "Unimplemented" primary template declared noreturn that throws >>> or exits but whose specializations return a useful value and >>> make use of attribute malloc (or one of the other blacklisted >>> attributes). >>> >>> 2) The converse of (1). A primary that returns a useful malloc >>> like value and some of whose specializations are not/cannot >>> be meaningfully implemented and are declared noreturn. >> >> Right, but I still disagree with this use of noreturn, and therefore >> don't consider these cases important. >> >>> Defining template specializations that differ from the primary >>> template in their implementation is idiomatic (analogous to >>> defining overloads or overridden virtual functions). >> >>> In any event, I am mainly interested in fixing the two bugs >>> (one a P1 regression). If you consider changing the warning >>> aspect of the patch a condition of accepting the fix please let >>> me know. Removing the noreturn keyword from the whitelist is >>> trivial. >> >> Please do. > > Attached is an updated patch with this change and with > the TREE_HAS_VOLATILE bit split up between types and functions. > I've also removed warn_unused_result from the blacklist (see > below). > > Bootstrapped on x864_64, regression test is in progress. > > Martin > > While reviewing other related bugs I noticed 83502. This patch > doesn't fix the first test case in the bug (attribute noinline > vs always_inline). Somehow those are still copied from > the primary to the specialization and can cause conflicts. Hmm, that's odd. Why is that? > It does fix the second test case but with the noreturn change > it would issue a bogus -Wmissing-attributes warning for the > explicit specialization below. Adding the warn_unused_result > attribute to it would then make GCC complain about a conflict > between the added attribute and noreturn, while removing it > would lead to worse code. > > template <class T> > int __attribute__ ((warn_unused_result)) f (T) { return 0; } > > template <> > int __attribute__ ((noreturn)) f<int> (int) { throw 0; } > > void fi () { f (0); } I continue to disagree with this use of attribute noreturn. > + /* Merge the function-has-side-effects bit. */ > + if (TREE_THIS_VOLATILE (newdecl)) > + TREE_THIS_VOLATILE (olddecl) = 1; > + > + if (merge_attr) TREE_THIS_VOLATILE means attribute noreturn, not whether the function has side-effects; it should be handled in the blocks controlled by merge_attr. Jason
>> While reviewing other related bugs I noticed 83502. This patch >> doesn't fix the first test case in the bug (attribute noinline >> vs always_inline). Somehow those are still copied from >> the primary to the specialization and can cause conflicts. > > Hmm, that's odd. Why is that? Because duplicate_decl calls diagnose_mismatched_attributes() on the NEWDECL and OLDDECL. (Attribute optimize would do the same thing.) I was trying to keep the fix small but it makes sense to take care of this as well so I have in this revision. >> It does fix the second test case but with the noreturn change >> it would issue a bogus -Wmissing-attributes warning for the >> explicit specialization below. Adding the warn_unused_result >> attribute to it would then make GCC complain about a conflict >> between the added attribute and noreturn, while removing it >> would lead to worse code. >> >> template <class T> >> int __attribute__ ((warn_unused_result)) f (T) { return 0; } >> >> template <> >> int __attribute__ ((noreturn)) f<int> (int) { throw 0; } >> >> void fi () { f (0); } > > I continue to disagree with this use of attribute noreturn. >> + /* Merge the function-has-side-effects bit. */ >> + if (TREE_THIS_VOLATILE (newdecl)) >> + TREE_THIS_VOLATILE (olddecl) = 1; >> + >> + if (merge_attr) > > TREE_THIS_VOLATILE means attribute noreturn, not whether the function > has side-effects; it should be handled in the blocks controlled by > merge_attr. Whoops. That was a silly goof. I must have misread the comment above the macro definition. I also didn't have a test for it (or some of the other changes I've made) so I didn't see the problem. Attached is an enhanced version of the patch that handles (and tests) more of the commonly used attributes. I'm not sure why in the merge_attr block I have to merge TREE_THIS_VOLATILE and TREE_NOTHROW back and forth but not also READONLY, PURE, or MALLOC, but without it tests fail. Martin PS Would it be possible to add a new macro with "noreturn" in the name to make it more intuitive? (And ditto perhaps also for TREE_READONLY for "const" functions, though for whatever reason that seems easier to decipher. I know you're all used to it but it's far from intuitive.) PPS Duplicate_decls is over 1,400 lines long. If there is more work to do here in stage 1 (I suspect there might be), would you mind if I broke it up into two or more, say one for functions, another for types, or whatever grouping makes most sense to make it easier to follow? PR c++/83871 - wrong code for attribute const and pure on distinct template specializations PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization gcc/ChangeLog: PR c++/83871 * gcc/doc/invoke.texi (-Wmissing-attributes): New option. gcc/c-family/ChangeLog: PR c++/83871 * c.opt (-Wmissing-attributes): New option. gcc/cp/ChangeLog: PR c++/83871 PR c++/83503 * cp-tree.h (warn_spec_missing_attributes): New function. ((check_explicit_specialization): Add an argument. Call the above function. * decl.c (duplicate_decls): Avoid applying primary function template's attributes to its explicit specializations. cp/pt.c (warn_spec_missing_attributes): Define. gcc/testsuite/ChangeLog: PR c++/83871 PR c++/83503 * g++.dg/Wmissing-attributes.C: New test. * g++.dg/ext/attr-const-pure.C: New test. * g++.dg/ext/attr-const.C: New test. * g++.dg/ext/attr-deprecated-2.C: New test. * g++.dg/ext/attr-malloc-2.C: New test. * g++.dg/ext/attr-malloc.C: New test. * g++.dg/ext/attr-noinline-2.C: New test. * g++.dg/ext/attr-noinline.C: New test. * g++.dg/ext/attr-nonnull.C: New test. * g++.dg/ext/attr-noreturn-2.C: New test. * g++.dg/ext/attr-noreturn.C: New test. * g++.dg/ext/attr-nothrow-2.C: New test. * g++.dg/ext/attr-nothrow.C: New test. * g++.dg/ext/attr-optimize.C: New test. * g++.dg/ext/attr-pure.C: New test. * g++.dg/ext/attr-returns-nonnull.C: New test. * g++.dg/ext/attr-warning.C: New test. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 421b146..a4c8c8f 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -781,6 +781,11 @@ Wtemplates C++ ObjC++ Var(warn_templates) Warning Warn on primary template declaration. +Wmissing-attributes +C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +Warn about declarations of entities that may be missing attributes +that related entities have been declared with it. + Wmissing-format-attribute C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format) ; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index aef022f..abcd1a6 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6463,7 +6463,8 @@ extern void end_specialization (void); extern void begin_explicit_instantiation (void); extern void end_explicit_instantiation (void); extern void check_unqualified_spec_or_inst (tree, location_t); -extern tree check_explicit_specialization (tree, tree, int, int); +extern tree check_explicit_specialization (tree, tree, int, int, + tree = NULL_TREE); extern int num_template_headers_for_class (tree); extern void check_template_variable (tree); extern tree make_auto (void); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index cf91773..b922493 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -1405,9 +1405,18 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) " literal operator template %qD", newdecl, olddecl); } + /* True to merge attributes between the declarations, false to + set OLDDECL's attributes to those of NEWDECL (for template + explicit specializations that specify their own attributes + independent of those specified for the primary template). */ + const bool merge_attr = (TREE_CODE (newdecl) != FUNCTION_DECL + || !DECL_TEMPLATE_SPECIALIZATION (newdecl) + || DECL_TEMPLATE_SPECIALIZATION (olddecl)); + if (DECL_P (olddecl) && TREE_CODE (newdecl) == FUNCTION_DECL && TREE_CODE (olddecl) == FUNCTION_DECL + && merge_attr && diagnose_mismatched_attributes (olddecl, newdecl)) { if (DECL_INITIAL (olddecl)) @@ -1969,10 +1978,13 @@ next_arg:; DECL_ORIGINAL_TYPE (newdecl) = DECL_ORIGINAL_TYPE (olddecl); } - /* Copy all the DECL_... slots specified in the new decl - except for any that we copy here from the old type. */ - DECL_ATTRIBUTES (newdecl) - = (*targetm.merge_decl_attributes) (olddecl, newdecl); + /* Copy all the DECL_... slots specified in the new decl except for + any that we copy here from the old type. */ + if (merge_attr) + DECL_ATTRIBUTES (newdecl) + = (*targetm.merge_decl_attributes) (olddecl, newdecl); + else + DECL_ATTRIBUTES (olddecl) = DECL_ATTRIBUTES (newdecl); if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl)) { @@ -2099,9 +2111,10 @@ next_arg:; } } } - else - /* Merge the data types specified in the two decls. */ + else if (merge_attr) newtype = merge_types (TREE_TYPE (newdecl), TREE_TYPE (olddecl)); + else + newtype = TREE_TYPE (newdecl); if (VAR_P (newdecl)) { @@ -2165,14 +2178,6 @@ next_arg:; && !(processing_template_decl && uses_template_parms (newdecl))) layout_decl (newdecl, 0); - /* Merge the type qualifiers. */ - if (TREE_READONLY (newdecl)) - TREE_READONLY (olddecl) = 1; - if (TREE_THIS_VOLATILE (newdecl)) - TREE_THIS_VOLATILE (olddecl) = 1; - if (TREE_NOTHROW (newdecl)) - TREE_NOTHROW (olddecl) = 1; - /* Merge deprecatedness. */ if (TREE_DEPRECATED (newdecl)) TREE_DEPRECATED (olddecl) = 1; @@ -2190,6 +2195,15 @@ next_arg:; DECL_FUNCTION_SPECIFIC_OPTIMIZATION (newdecl) = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (olddecl); } + else + { + /* Merge the const type qualifier. */ + if (TREE_READONLY (newdecl)) + TREE_READONLY (olddecl) = 1; + /* Merge the volatile type qualifier. */ + if (TREE_THIS_VOLATILE (newdecl)) + TREE_THIS_VOLATILE (olddecl) = 1; + } /* Merge the initialization information. */ if (DECL_INITIAL (newdecl) == NULL_TREE @@ -2209,14 +2223,29 @@ next_arg:; DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (newdecl) |= DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (olddecl); DECL_NO_LIMIT_STACK (newdecl) |= DECL_NO_LIMIT_STACK (olddecl); - TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); - TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); - DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); - DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); - TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); - DECL_LOOPING_CONST_OR_PURE_P (newdecl) + DECL_LOOPING_CONST_OR_PURE_P (newdecl) |= DECL_LOOPING_CONST_OR_PURE_P (olddecl); + + if (merge_attr) + { + TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); + TREE_THIS_VOLATILE (olddecl) |= TREE_THIS_VOLATILE (newdecl); + TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); + TREE_NOTHROW (olddecl) |= TREE_NOTHROW (newdecl); + TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); + DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); + DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); + } + else + { + /* Merge the noreturn bit. */ + TREE_THIS_VOLATILE (olddecl) = TREE_THIS_VOLATILE (newdecl); + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); + DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl); + } /* Keep the old RTL. */ COPY_DECL_RTL (olddecl, newdecl); } @@ -2381,17 +2410,33 @@ next_arg:; /* [temp.expl.spec/14] We don't inline explicit specialization just because the primary template says so. */ - /* But still keep DECL_DISREGARD_INLINE_LIMITS in sync with - the always_inline attribute. */ - if (DECL_DISREGARD_INLINE_LIMITS (olddecl) - && !DECL_DISREGARD_INLINE_LIMITS (newdecl)) + if (merge_attr) + { + /* But still keep DECL_DISREGARD_INLINE_LIMITS in sync with + the always_inline attribute. */ + if (DECL_DISREGARD_INLINE_LIMITS (olddecl) + && !DECL_DISREGARD_INLINE_LIMITS (newdecl)) + { + if (DECL_DECLARED_INLINE_P (newdecl)) + DECL_DISREGARD_INLINE_LIMITS (newdecl) = true; + else + DECL_ATTRIBUTES (newdecl) + = remove_attribute ("always_inline", + DECL_ATTRIBUTES (newdecl)); + } + } + else { if (DECL_DECLARED_INLINE_P (newdecl)) DECL_DISREGARD_INLINE_LIMITS (newdecl) = true; - else - DECL_ATTRIBUTES (newdecl) - = remove_attribute ("always_inline", - DECL_ATTRIBUTES (newdecl)); + + DECL_DECLARED_INLINE_P (olddecl) + = DECL_DECLARED_INLINE_P (newdecl); + + DECL_DISREGARD_INLINE_LIMITS (olddecl) + = DECL_DISREGARD_INLINE_LIMITS (newdecl); + + DECL_UNINLINABLE (olddecl) = DECL_UNINLINABLE (newdecl); } } else if (new_defines_function && DECL_INITIAL (olddecl)) @@ -8917,7 +8962,8 @@ grokfndecl (tree ctype, template_count, 2 * funcdef_flag + 4 * (friendp != 0) + - 8 * concept_p); + 8 * concept_p, + *attrlist); if (decl == error_mark_node) return NULL_TREE; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 42fd872..9c2e5e6 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. If not see all methods must be provided in header files; can't use a source file that contains only the method templates and "just win". */ +#include <string> #include "config.h" #include "system.h" #include "coretypes.h" @@ -1473,8 +1474,10 @@ is_specialization_of_friend (tree decl, tree friend_decl) /* Register the specialization SPEC as a specialization of TMPL with the indicated ARGS. IS_FRIEND indicates whether the specialization - is actually just a friend declaration. Returns SPEC, or an - equivalent prior declaration, if available. + is actually just a friend declaration. ATTRLIST is the list of + attributes that the specialization is declared with or NULL when + it isn't. Returns SPEC, or an equivalent prior declaration, if + available. We also store instantiations of field packs in the hash table, even though they are not themselves templates, to make lookup easier. */ @@ -2615,6 +2618,110 @@ check_unqualified_spec_or_inst (tree t, location_t loc) } } +/* Warn for a template specialization SPEC that is missing some of a set + of function or type attributes that the template TEMPL is declared with. + ATTRLIST is a list of additional attributes that SPEC should be taken + to ultimately be declared with. */ + +static void +warn_spec_missing_attributes (tree tmpl, tree spec, tree attrlist) +{ + if (DECL_FUNCTION_TEMPLATE_P (tmpl)) + tmpl = DECL_TEMPLATE_RESULT (tmpl); + + if (TREE_CODE (tmpl) != FUNCTION_DECL) + return; + + /* Avoid warning if either declaration or its type is deprecated. */ + if (TREE_DEPRECATED (tmpl) + || TREE_DEPRECATED (spec)) + return; + + tree tmpl_type = TREE_TYPE (tmpl); + tree spec_type = TREE_TYPE (spec); + + if (TREE_DEPRECATED (tmpl_type) + || TREE_DEPRECATED (spec_type) + || TREE_DEPRECATED (TREE_TYPE (tmpl_type)) + || TREE_DEPRECATED (TREE_TYPE (spec_type))) + return; + + tree tmpl_attrs[] = { DECL_ATTRIBUTES (tmpl), TYPE_ATTRIBUTES (tmpl_type) }; + tree spec_attrs[] = { DECL_ATTRIBUTES (spec), TYPE_ATTRIBUTES (spec_type) }; + + if (!spec_attrs[0]) + spec_attrs[0] = attrlist; + else if (!spec_attrs[1]) + spec_attrs[1] = attrlist; + + /* Avoid warning if the primary has no attributes. */ + if (!tmpl_attrs[0] && !tmpl_attrs[1]) + return; + + /* Avoid warning if either declaration contains an attribute on + the white list below. */ + const char* const whitelist[] = { + "error", "warning" + }; + + for (unsigned i = 0; i != 2; ++i) + for (unsigned j = 0; j != sizeof whitelist / sizeof *whitelist; ++j) + if (lookup_attribute (whitelist[j], tmpl_attrs[i]) + || lookup_attribute (whitelist[j], spec_attrs[i])) + return; + + /* Avoid warning if the difference between the primary and + the specialization is not in one of the attributes below. */ + const char* const blacklist[] = { + "alloc_align", "alloc_size", "assume_aligned", "format", + "format_arg", "malloc", "nonnull" + }; + + /* Put together a list of the black listed attributes that the primary + template is declared with that the specialization is not, in case + it's not apparent from the most recent declaration of the primary. */ + unsigned nattrs = 0; + std::string str; + + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) + { + for (unsigned j = 0; j != 2; ++j) + { + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) + continue; + + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) + { + if (lookup_attribute (blacklist[i], spec_attrs[k])) + break; + + if (str.size ()) + str += ", "; + str += "%<"; + str += blacklist[i]; + str += "%>"; + ++nattrs; + } + } + } + + if (!nattrs) + return; + + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attributes, + "explicit specialization %q#D may be missing attributes", + spec)) + { + if (nattrs > 1) + str = G_("missing primary template attributes ") + str; + else + str = G_("missing primary template attribute ") + str; + + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); + } + +} + /* Check to see if the function just declared, as indicated in DECLARATOR, and in DECL, is a specialization of a function template. We may also discover that the declaration is an explicit @@ -2656,7 +2763,8 @@ tree check_explicit_specialization (tree declarator, tree decl, int template_count, - int flags) + int flags, + tree attrlist) { int have_def = flags & 2; int is_friend = flags & 4; @@ -3113,8 +3221,13 @@ check_explicit_specialization (tree declarator, it again. Partial specializations will be registered in process_partial_specialization. */ if (!processing_template_decl) - decl = register_specialization (decl, gen_tmpl, targs, - is_friend, 0); + { + warn_spec_missing_attributes (gen_tmpl, decl, attrlist); + + decl = register_specialization (decl, gen_tmpl, targs, + is_friend, 0); + } + /* A 'structor should already have clones. */ gcc_assert (decl == error_mark_node diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index e70e2ba..8d366c6 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -295,7 +295,7 @@ Objective-C and Objective-C++ Dialects}. -Winvalid-pch -Wlarger-than=@var{len} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol --Wmisleading-indentation -Wmissing-braces @gol +-Wmisleading-indentation -Wmissing-attributes -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-include-dirs @gol -Wno-multichar -Wmultistatement-macros -Wnonnull -Wnonnull-compare @gol -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol @@ -3928,6 +3928,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. -Wmemset-elt-size @gol -Wmemset-transposed-args @gol -Wmisleading-indentation @r{(only for C/C++)} @gol +-Wmissing-attributes @gol -Wmissing-braces @r{(only for C/ObjC)} @gol -Wmultistatement-macros @gol -Wnarrowing @r{(only for C++)} @gol @@ -4591,6 +4592,36 @@ about the layout of the file that the directive references. This warning is enabled by @option{-Wall} in C and C++. +@item -Wmissing-attributes +@opindex Wmissing-attributes +@opindex Wno-missing-attributes +Warn when a declaration of a function is missing one or more attributes +that a related function is declared with and whose absence may adversely +affect the correctness or efficiency of generated code. For example, in +C++, the warning is issued when an explicit specialization of a primary +template declared with attribute @code{alloc_align}, @code{alloc_size}, +@code{assume_aligned}, @code{format}, @code{format_arg}, @code{malloc}, +or @code{nonnull} is declared without it. Attributes @code{deprecated}, +@code{error}, and @code{warning} suppress the warning. +(@pxref{Function Attributes}). + +@option{-Wmissing-attributes} is enabled by @option{-Wall}. + +For example, since the declaration of the primary function template +below makes use of both attribute @code{malloc} and @code{alloc_size} +the declaration of the explicit specialization of the template is +diagnosed because it is missing one of the attributes. + +@smallexample +template <class T> +T* __attribute__ ((malloc, alloc_size (1))) +allocate (size_t); + +template <> +void* __attribute__ ((malloc)) // missing alloc_size +allocate<void> (size_t); +@end smallexample + @item -Wmissing-braces @opindex Wmissing-braces @opindex Wno-missing-braces diff --git a/gcc/print-tree.c b/gcc/print-tree.c index cba8bac..caf5f26 100644 --- a/gcc/print-tree.c +++ b/gcc/print-tree.c @@ -377,6 +377,8 @@ print_node (FILE *file, const char *prefix, tree node, int indent, fputs (" function-specific-opt", file); if (code == FUNCTION_DECL && DECL_DECLARED_INLINE_P (node)) fputs (" autoinline", file); + if (code == FUNCTION_DECL && DECL_UNINLINABLE (node)) + fputs (" uninlinable", file); if (code == FUNCTION_DECL && DECL_BUILT_IN (node)) fputs (" built-in", file); if (code == FUNCTION_DECL && DECL_STATIC_CHAIN (node)) diff --git a/gcc/testsuite/g++.dg/Wmissing-attributes.C b/gcc/testsuite/g++.dg/Wmissing-attributes.C new file mode 100644 index 0000000..f4ebce1 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wmissing-attributes.C @@ -0,0 +1,102 @@ +// PR c++/83871 - wrong code for attribute const and pure on distinct +// template specializations +// Test to verify that a declaration of an explicit specialization with +// no attributes is diagnosed when the primary template is declared with +// one or more attributes. The warning helps highlight a change in GCC +// 8 from previous versions that copied the attributes from the primary +// to the specialization. It also helps point out simply forgetting to +// declare the specialization with an attribute. +// { dg-do compile } +// { dg-options "-Wmissing-attributes" } + +#define ATTR(list) __attribute__ (list) + + +// Verify that a primary without attributes doesn't cause warnings. +template <class T> void fnoattr (); + +template <> void fnoattr<void>(); +template <> void ATTR ((cold)) fnoattr<int>(); +template <> void ATTR ((hot)) fnoattr<double>(); + +// Verify that a noreturn primary also doesn't cause warnings. +template <class T> int ATTR ((noreturn)) fnoreturn (); + +template <> int fnoreturn<void>(); +template <> int ATTR ((cold)) fnoreturn<int>(); +template <> int ATTR ((hot)) fnoreturn<double>(); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_all (int); // { dg-message "missing primary template attributes \(.malloc., .alloc_size.|.alloc_size., .malloc.\)" } + +template <> +void* +missing_all<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +// Verify that specifying the same attributes in whatever order +// doesn't trigger the warning, even when other attributes are +// added. +template <> +void* +ATTR ((alloc_size (1), malloc)) +missing_all<char>(int); + +template <> +void* +ATTR ((alloc_size (1))) ATTR ((malloc)) ATTR ((returns_nonnull)) +missing_all<char>(int); // T = char, same as above + +template <> +void* +ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc)) +missing_all<char>(int); // T = char, same as above + +// Verify that the following attributes suppress the warning. +template <> void* ATTR ((error (""))) missing_all<short>(int); +template <> void* ATTR ((deprecated)) missing_all<int>(int); +template <> void* ATTR ((warning (""))) missing_all<double>(int); + + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_malloc (int); // { dg-message "missing primary template attribute .malloc." } + +template <> +void* +ATTR ((alloc_size (1))) +missing_malloc<char>(int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((malloc, alloc_size (1))) missing_malloc<short>(int); +template <> void* ATTR ((deprecated)) missing_malloc<int>(int); +template <> void* ATTR ((error (""))) missing_malloc<long>(int); +template <> void* ATTR ((warning (""))) missing_malloc<double>(int); + +template <class T> +void* +ATTR ((malloc, alloc_size (1))) +missing_alloc_size (int, int); // { dg-message "missing primary template attribute .alloc_size." } + +template <> +void* +ATTR ((malloc)) +missing_alloc_size<char>(int, int); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + + +template <class T> +void* +ATTR ((nonnull (1))) +missing_nonnull (void*); // { dg-message "missing primary template attribute .nonnull." } + +template <> +void* +ATTR ((malloc)) +missing_nonnull<char>(void*); // { dg-warning "explicit specialization .\[^\n\r\]+. may be missing attributes" } + +template <> void* ATTR ((nonnull (1))) missing_nonnull<short>(void*); +template <> void* ATTR ((deprecated)) missing_nonnull<int>(void*); +template <> void* ATTR ((error (""))) missing_nonnull<long>(void*); +template <> void* ATTR ((warning (""))) missing_nonnull<double>(void*); diff --git a/gcc/testsuite/g++.dg/ext/attr-const-pure.C b/gcc/testsuite/g++.dg/ext/attr-const-pure.C new file mode 100644 index 0000000..f7c6f3b --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const-pure.C @@ -0,0 +1,144 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +int global; + +template <class T> +int __attribute__ ((pure)) +f (T); + +template <> +int __attribute__ ((const)) f<int> (int); // { dg-bogus "ignoring attribute .const." } + +void f_pure_primary_elim (); +void f_pure_primary_keep (); +void f_const_spec_elim (); + +void call_pure_primary_elim (double x) +{ + // Only the first call to f(x) must be emitted, the second one + // is expected to be eliminated because the primary template + // is pure. + int i0 = f (x); + int i1 = f (x); + if (i0 != i1) + f_pure_primary_elim (); +} + +void call_pure_primary_keep (const char *s) +{ + // Both calls to f(x) must be emitted because the primary is + // pure and may read global. + int i0 = f (s); + global = 123; + int i1 = f (s); + if (i0 != i1) + f_pure_primary_keep (); +} + +void call_const_spec_elim (int i) +{ + // Only the first call to f(x) must be emitted, the second + // one is expected to be eliminated again, this time because + // unlike the pure primary, the specialization is const. + int i0 = f (i); + global = 123; + int i1 = f (i); + if (i0 != i1) + f_const_spec_elim (); +} + +template <class T> +int __attribute__ ((const)) +g (T); + +template <> +int __attribute__ ((pure)) g<int> (int); // { dg-bogus "ignoring attribute .const." } + +template <class T> +int __attribute__ ((const)) +h (T); + +template <class T> +int __attribute__ ((pure)) +h (const T*); + +template <> +int h<int> (int); + +template <> +int h<int*> (int*); + +extern void h_const_primary_elim (); +extern void h_pure_cstptr_elim (); +extern void h_cstptr_keep (); +extern void h_int_keep (); +extern void h_intptr_keep (); + +void call_const_primary_elim (double x) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. + int i0 = h (x); + int i1 = h (x); + + if (i0 != i1) // must be folded into false + h_const_primary_elim (); // must be eliminated +} + +void call_pure_cstptr_elim (const void *p) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. This verifies that h<const + // void*>*() is treated as const in this context. + int i0 = h (p); + int i1 = h (p); + + if (i0 != i1) // must be folded into false + h_pure_cstptr_elim (); // must be eliminated +} + +void call_cstptr_keep (const void *p) +{ + // Because of the store to the global, both calls to h(p) must + // be emitted. This verifies that h<const void*>*() is not + // treated as const. + int i0 = h (p); + global = 123; + int i1 = h (p); + + if (i0 != i1) // must not be folded + h_cstptr_keep (); // must be emitted +} + +void call_int_keep (int i) +{ + // Both calls to h(i) must be emitted. + int i0 = h (i); + int i1 = h (i); + + if (i0 != i1) // must not be folded + h_int_keep (); // must be emitted +} + +void call_intptr_keep (int *ip) +{ + // Both calls to h(ip) must be emitted. + int i0 = h (ip); + int i1 = h (ip); + + if (i0 != i1) // must not be folded + h_intptr_keep (); // must be emitted +} + +// { dg-final { scan-tree-dump-not "f_pure_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "f_const_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "f_const_spec_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "h_pure_cstptr_elim" "optimized" } } + +// { dg-final { scan-tree-dump-times "f_pure_primary_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_cstptr_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_int_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_intptr_keep" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-const.C b/gcc/testsuite/g++.dg/ext/attr-const.C new file mode 100644 index 0000000..a9884db --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const.C @@ -0,0 +1,69 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + { dg-do compile } + { dg-options "-O -Wall" } */ + +int __attribute__ ((const)) fconst_none (); +int fconst_none (); + +void test_const_none_failed (); + +void func_const_none () +{ + int i0 = fconst_none (); + int i1 = fconst_none (); + if (i0 != i1) + test_const_none_failed (); + + // { dg-final { scan-tree-dump-not "test_const_none_failed" "optimized" } } +} + + +int fnone_const (); +int __attribute__ ((const)) fnone_const (); + +void test_none_const_failed (); + +void func_none_const () +{ + int i0 = fnone_const (); + int i1 = fnone_const (); + if (i0 != i1) + test_none_const_failed (); + + // { dg-final { scan-tree-dump-not "test_none_const_failed" "optimized" } } +} + + +template <class T> +int __attribute__ ((const)) fconst_none (T); + +template <class T> +int fconst_none (T); + +void template_const_none () +{ + int i0 = fconst_none<int> (0); + int i1 = fconst_none<int> (0); + if (i0 != i1) + test_const_none_failed (); + + // { dg-final { scan-tree-dump-not "test_const_none_failed" "optimized" } } +} + + +template <class T> +int fnone_const (T); + +template <class T> +int __attribute__ ((const)) fnone_const (T); + +void test_fnone_const () +{ + int i0 = fnone_const<int> (0); + int i1 = fnone_const<int> (0); + if (i0 != i1) + test_none_const_failed (); + + // { dg-final { scan-tree-dump-not "test_none_const_failed" "optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-deprecated-2.C b/gcc/testsuite/g++.dg/ext/attr-deprecated-2.C new file mode 100644 index 0000000..bf90415 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-deprecated-2.C @@ -0,0 +1,35 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute deprecated from a primary template declared +// with it. +// { dg-do compile } +// { dg-options "-Wall -fdump-tree-optimized" } + +struct Special; + +template <class T> +void fdeprecated_primary (); + +// The primary isn't deprecated at this point so the declaration +// of its specialization should not be diagnosed. +template <> +void fdeprecated_primary<Special> (); // { dg-bogus "deprecated" } + +template <class T> +void __attribute__ ((deprecated)) +fdeprecated_primary (); + +void use_primary () +{ + // Verify that uses of the now deprecacted primary are diagnosed. + fdeprecated_primary<void>(); // { dg-warning "deprecated" "bug 84542" { xfail *-*-* } } + fdeprecated_primary<int>(); // { dg-warning "deprecated" "bug 84542" { xfail *-*-* } } +} + +void use_special () +{ + // Verify that the use of the non-deprecated specializatoin + // is not diagnosed. + fdeprecated_primary<Special>(); +} diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc-2.C b/gcc/testsuite/g++.dg/ext/attr-malloc-2.C new file mode 100644 index 0000000..600d430 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc-2.C @@ -0,0 +1,49 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that attribute malloc on multiple declarations of +// the same ordinary function are merged. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +void* __attribute__ ((malloc)) +fmalloc_none (unsigned); + +void* +fmalloc_none (unsigned); + +static char a[8]; + +void fmalloc_none_failed (); + +void test_fmalloc_none (void) +{ + void *p = fmalloc_none (1); + if (!p) + return; + + if (p == a) // must be false + fmalloc_none_failed (); // should be eliminated + + // Verify that the call to fmalloc_none() is eliminated. + // { dg-final { scan-tree-dump-not "fmalloc_none_failed" "optimized" } } +} + +void* fnone_malloc (unsigned); + +void* __attribute__ ((malloc)) +fnone_malloc (unsigned); + +void fnone_malloc_failed (); + +void test_fnone_malloc (void) +{ + void *p = fnone_malloc (1); + if (!p) + return; + + if (p == a) // must be false + fnone_malloc_failed (); // should be eliminated + + // Verify that the call to fnone_malloc() is eliminated. + // { dg-final { scan-tree-dump-not "fnone_malloc_failed" "optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc.C b/gcc/testsuite/g++.dg/ext/attr-malloc.C new file mode 100644 index 0000000..3cbb414 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc.C @@ -0,0 +1,69 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that an explicit template specifialization does not +// "inherit" attribute malloc from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class> +void* __attribute__ ((malloc)) +fmalloc (unsigned); + +template <> +void* +fmalloc<int>(unsigned); // { dg-warning "may be missing attributes" } + +static char a[8]; + +void fmalloc_void_malloc (); +void fmalloc_int_not_malloc (); + +void test_fmalloc_primary (void) +{ + void *p = fmalloc<void>(1); + if (!p) + return; + + if (p == a) // must be false + fmalloc_void_malloc (); // should be eliminated + + // Verify that the call to fmalloc_void_malloc() is eliminated. + // { dg-final { scan-tree-dump-not "fmalloc_void_malloc" "optimized" } } +} + + +void test_fmalloc_spec_none (void) +{ + void *p = fmalloc<int>(1); + if (!p) + return; + + if (p == a) // can be true + fmalloc_int_not_malloc (); // must not be eliminated + + // Verify that the call to fmalloc_int_not_malloc() is retained. + // { dg-final { scan-tree-dump-times "fmalloc_int_not_malloc" 1 "optimized" } } +} + +template <> +void* +fmalloc<long>(unsigned); // { dg-warning "may be missing attributes" } + +template <> +void* __attribute__ ((malloc)) +fmalloc<long>(unsigned); + +void fmalloc_long_malloc (); + +void test_fmalloc_spec_malloc (void) +{ + void *p = fmalloc<long>(1); + if (!p) + return; + + if (p == a) // can be true + fmalloc_long_malloc (); // must not be eliminated + + // Verify that the call to fmalloc_long_malloc() is eliminated. + // { dg-final { scan-tree-dump-not "fmalloc_long_malloc" "optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-noinline-2.C b/gcc/testsuite/g++.dg/ext/attr-noinline-2.C new file mode 100644 index 0000000..4aab4f1 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-noinline-2.C @@ -0,0 +1,73 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attributes always_inline or noinline from a primary template +// declared with either. Unlike attr-noinline.C, this test enables +// optimization to verify that noinline prevents inlining. +// { dg-do compile } +// { dg-options "-O2 -Wall -fdump-tree-optimized" } + +enum Special { }; + +int global; + +template <class T> +inline void __attribute__ ((always_inline)) +falways_inline_noinline () +{ + // Create a side-effect that's unique to this function. + global = __LINE__; +} + +template <> +void __attribute__ ((noinline)) +falways_inline_noinline<Special>() +{ + global = __LINE__; +} + +// Verify that a call to the primary is inlined but one to +// the explicit specialization is not. + +void test_elim_primary_1 (void) +{ + // Should be inlined. + falways_inline_noinline<void>(); +// { dg-final { scan-tree-dump-not "falways_inline_noinline<void> *\\(\\)" "optimized" } } +} + +void test_keep_special_1 (void) +{ + // Should not be inlined. + falways_inline_noinline<Special>(); +// { dg-final { scan-tree-dump-times "falways_inline_noinline<Special> *\\(\\);" 1 "optimized" } } +} + + +template <class T> +void __attribute__ ((noinline)) +fnoinline_always_inline () +{ + global = __LINE__; +} + +template <> +inline void __attribute__ ((always_inline)) +fnoinline_always_inline<Special>() // { dg-bogus "follows declaration" } +{ + global = __LINE__; +} + +void test_keep_primary_2 (void) +{ + // Should not be inlined. + fnoinline_always_inline<void>(); +// { dg-final { scan-tree-dump-times "fnoinline_always_inline<void> *\\(\\);" 1 "optimized" } } +} + +void test_elim_special_2 (void) +{ + // Should be inlined. + fnoinline_always_inline<Special>(); +// { dg-final { scan-tree-dump-not "fnoinline_always_inline<Special> *\\(\\);" optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-noinline.C b/gcc/testsuite/g++.dg/ext/attr-noinline.C new file mode 100644 index 0000000..b09037b --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-noinline.C @@ -0,0 +1,128 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attributes always_inline or noinline from a primary template +// declared with either. The test disables optimization to verify that +// always_inline forces inlining. +// { dg-do compile } +// { dg-options "-O0 -Wall -fdump-tree-optimized" } + +enum Special { }; + +template <class T> +inline void __attribute__ ((always_inline)) +falways_inline_none () +{ + // Primary template should always be inlined, even without optimization. + asm (""); // induce a no-op "side-effect" +} + +template <> +inline void +falways_inline_none<Special>() +{ + // The specialization should not be inlined without optimization, even + // though it's declared inline. + asm (""); +} + +// Verify that a call to the primary is inlined but one to +// the explicit specialization is not. + +void test_elim_primary_1 (void) +{ + // Should be inlined. + falways_inline_none<void>(); +// { dg-final { scan-tree-dump-not "falways_inline_none<void> *\\(\\)" "optimized" } } +} + +void test_keep_special_1 (void) +{ + // Should not be inlined. + falways_inline_none<Special>(); +// { dg-final { scan-tree-dump-times "falways_inline_none<Special> *\\(\\);" 1 "optimized" } } +} + + +template <class T> +inline void __attribute__ ((always_inline)) +falways_inline_noinline () +{ + asm (""); // induce a no-op "side-effect" +} + +template <> +void __attribute__ ((noinline)) +falways_inline_noinline<Special>() { asm (""); } + +// Verify that a call to the primary is inlined but one to +// the explicit specialization is not. + +void test_elim_primary_2 (void) +{ + falways_inline_noinline<void>(); +// { dg-final { scan-tree-dump-not "falways_inline_noinline<void> *\\(\\)" "optimized" } } +} + +void test_keep_special_2 (void) +{ + falways_inline_noinline<Special>(); +// { dg-final { scan-tree-dump-times "falways_inline_noinline<Special> *\\(\\);" 1 "optimized" } } +} + + +template <class T> +inline void +fnone_always_inline () +{ + asm (""); // induce a no-op "side-effect" +} + +template <> +inline void __attribute__ ((always_inline)) +fnone_always_inline<Special>() { asm (""); } + +// Verify that a call to the primary is not inlined but one to +// the explicit specialization is. + +void test_keep_primary_3 (void) +{ + fnone_always_inline<void>(); +// { dg-final { scan-tree-dump-times "fnone_always_inline<void> *\\(\\);" 1 "optimized" } } +} + +void test_elim_special_3 (void) +{ + fnone_always_inline<Special>(); +// { dg-final { scan-tree-dump-not "fnone_always_inline<Special> *\\(\\);" optimized" } } +} + + +template <class T> +void __attribute__ ((noinline)) +fnoinline_always_inline () +{ + asm (""); // induce a no-op "side-effect" +} + +template <> +inline void __attribute__ ((always_inline)) +fnoinline_always_inline<Special>() // { dg-bogus "follows declaration" } +{ + asm (""); +} + +// Verify that a call to the primary is not inlined but one to +// the explicit specialization is. + +void test_keep_primary_4 (void) +{ + fnoinline_always_inline<void>(); +// { dg-final { scan-tree-dump-times "fnoinline_always_inline<void> *\\(\\);" 1 "optimized" } } +} + +void test_elim_special_4 (void) +{ + fnoinline_always_inline<Special>(); +// { dg-final { scan-tree-dump-not "fnoinline_always_inline<Special> *\\(\\);" optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-nonnull.C new file mode 100644 index 0000000..57d2cb0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nonnull.C @@ -0,0 +1,31 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nonnull (1))) +f (T*, T*, T*); + +template <> +void +f<int>(int*, int*, int*); // { dg-warning "may be missing attributes" } + +template <> +void __attribute__ ((nonnull (3))) +f<float>(float*, float*, float*); + + +void test_nonnull (void) +{ + f<void>(0, 0, 0); // { dg-warning "null argument where non-null required \\\(argument 1\\\)" } + + f<int>(0, 0, 0); // { dg-bogus "null argument" } + + f<float>(0, 0, 0); + // { dg-bogus "null argument where non-null required \\\(argument 1\\\)" "" { target *-*-* } .-1 } + // { dg-warning "null argument where non-null required \\\(argument 3\\\)" "" { target *-*-* } .-2 } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-noreturn-2.C b/gcc/testsuite/g++.dg/ext/attr-noreturn-2.C new file mode 100644 index 0000000..cf70ba1 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-noreturn-2.C @@ -0,0 +1,47 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + Test to verify that attributes noreturn on multiple declarations of + the same function are merged. + { dg-do compile } + { dg-options "-O -fdump-tree-eh" } */ + +int __attribute__ ((noreturn)) fnoreturn (); + +void fnoreturn_failed (); + +int test_noreturn () throw () +{ + fnoreturn (); + fnoreturn_failed (); + // Verify that the call to fnoreturn_failed() is eliminated. + // { dg-final { scan-tree-dump-not "fnoreturn_failed" "optimized" } } + + // Expect no -Wreturn-type warning despite the absence of a return + // statement in a non-void function. +} + + +int __attribute__ ((noreturn)) fnoreturn_none (); +int fnoreturn_none (); + +void fnoreturn_none_failed (); + + +int test_noreturn_none () +{ + fnoreturn_none (); + fnoreturn_none_failed (); + // { dg-final { scan-tree-dump-not "fnoreturn_none_failed" "optimized" } } +} + +int fnone_noreturn (); +int __attribute__ ((noreturn)) fnone_noreturn (); + +void fnone_noreturn_failed (); + +int test_none_noreturn () throw () +{ + fnone_noreturn (); + fnone_noreturn_failed (); + // { dg-final { scan-tree-dump-not "fnone_noreturn_failed" "optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-noreturn.C b/gcc/testsuite/g++.dg/ext/attr-noreturn.C new file mode 100644 index 0000000..7d053d8 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-noreturn.C @@ -0,0 +1,80 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute noreturn from a primary template declared with +// one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +struct Noreturn { }; +struct Returns { }; + +// Primary declared noreturn but explicit specialization is not. +template <class T> int __attribute__ ((noreturn)) f (); +template <> int f<Returns>(); + +// Explicit specialization is noreturn but primary is not. +template <class T> int g (); +template <> int __attribute__ ((noreturn)) g<Noreturn>(); + +int val; + +int test_primary_noreturn (char, short) +{ + // Only the first call should be emitted, the second one should + // be eliminated because the first one doesn't return. + val = f<char>() + f<short>(); +} // expect no -Wreturn-type warning here + +int test_noreturn (int) +{ + // Call should be retained. + f<int>(); +} // expect no -Wreturn-type warning here + +int test_special_return (int) +{ + // Both calls must be emitted. + int val = f<Returns>() + f<Returns>(); + (void)&val; +} // { dg-warning "no return statement in function returning non-void" } + + +int test_primary_return (void) +{ + int val = g<char>() + g<int>(); + (void)&val; +} // { dg-warning "no return statement in function returning non-void" } + + +int test_special_noreturn (int, long) +{ + g<Noreturn>(); +} // expect no -Wreturn-type warning here + + +// Verify that the call to f<short>() above is eliminated but the call +// to f<int>() and the two calls to f<Returns>() are retained. +// { dg-final { scan-tree-dump-not "f<short>" "optimized" } } +// { dg-final { scan-tree-dump-times "f<Returns>" 2 "optimized" } } + +// Verify that the second call to f<Returns>() in test_special_return() +// is followed by __builtin_unreachable() because there is no return +// statement in the function. +// { dg-final { scan-tree-dump-times "f<Returns> \\(\\);\[\n\r \]+__builtin_unreachable" 1 "optimized" } } + + +// Verify that the call to g<short>() above is eliminated but the call +// to g<char>() and to g<Noreturn>() are both retained. +// { dg-final { scan-tree-dump-not "g<short>" "optimized" } } +// { dg-final { scan-tree-dump-times "g<char>" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "g<Noreturn>" 1 "optimized" } } + +// Verify that the call to g<int>() in test_primary_return() is +// followed by __builtin_unreachable() because there is no return +// statement in the function. +// { dg-final { scan-tree-dump-times "g<int> *\\(\\);\[\n\r \]+__builtin_unreachable" 1 "optimized" } } +// Verify that the call to g<Noreturn>() in test_special_noreturn() +// is not followed by __builtin_unreachable() even though there is no +// return statement in the function. +// { dg-final { scan-tree-dump-times "g<Noreturn> *\\(\\);\[\n\r \]+__builtin_unreachable" 0 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C new file mode 100644 index 0000000..8f1d7af --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow-2.C @@ -0,0 +1,48 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + Test to verify that attributes nothrow on multiple declarations of + the same function are merged. + { dg-do compile } + { dg-options "-O -fdump-tree-eh" } */ + +void __attribute__ ((nothrow)) fnothrow (); + +void test_nothrow () throw () +{ + // No exception handling necessary around the call to fnothrow(). + fnothrow (); +} + +void __attribute__ ((nothrow)) fnothrow_none (); +void fnothrow_none (); + +void test_nothrow_none () throw () +{ + // No exception handling necessary around the call to fnothrow_none(). + fnothrow_none (); +} + +void fnone_nothrow (); +void __attribute__ ((nothrow)) fnone_nothrow (); + +void test_none_nothrow () throw () +{ + // No exception handling necessary around the call to fnone_nothrow(). + fnone_nothrow (); +} + +int __attribute__ ((nothrow)) fnothrow_noreturn_none (); +int __attribute__ ((noreturn)) fnothrow_noreturn_none (); +int fnothrow_noreturn_none (); + +int test_nothrow_noreturn_none () throw () +{ + // No exception handling necessary around the call(). + // No -Wreturn-value should be emitted because the function is + // declared noreturn. + fnothrow_noreturn_none (); +} + +// Verify that no exception handling code was emitted. +// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } } +// { dg-final { scan-tree-dump-not "resx" "eh" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow.C b/gcc/testsuite/g++.dg/ext/attr-nothrow.C new file mode 100644 index 0000000..1d3d715 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow.C @@ -0,0 +1,46 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute nothrow from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nothrow)) +f (); + +template <> +void f<int>(); + +void f_void_nothrow (); +void f_int_maythrow (); + +void fv (void) +{ + try + { + f<void>(); + } + catch (...) // cannot be be reached + { + f_void_nothrow (); // should be eliminated + } +} + + +void fi (void) +{ + try + { + f<int>(); + } + catch (...) // may be reached + { + f_int_maythrow (); // must not be eliminated + } +} + +// Verify that the call to f_void_nothrow() is eliminated but +// the call to f_int_maythrow() is retained. +// { dg-final { scan-tree-dump-not "f_void_nothrow" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_maythrow" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-optimize.C b/gcc/testsuite/g++.dg/ext/attr-optimize.C new file mode 100644 index 0000000..481cf3f --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-optimize.C @@ -0,0 +1,46 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute optimize from a primary template declared with +// one. +// { dg-do compile } +// { dg-options "-O2 -Wall -fdump-tree-optimized" } + +enum Special { }; + +void foptimize_none_primary_failed (); + +template <class T> +void __attribute__ ((optimize ("no-printf-return-value"))) +foptimize_none () +{ + // The call to snprintf and the test should be retained. + if (2 != __builtin_snprintf (0, 0, "%hhx", 0x12)) + foptimize_none_primary_failed (); +} + +void foptimize_none_special_failed (); + +template <> +inline void +foptimize_none<Special>() +{ + // The whole if statement should be eliminated. + if (3 != __builtin_snprintf (0, 0, "1%hhx", 0x12)) + foptimize_none_special_failed (); +} + +void test_primary () +{ + foptimize_none<void>(); + // { dg-final { scan-tree-dump-times "foptimize_none_primary_failed *\\(\\)" 1 "optimized" } } +} + +void test_special () +{ + // Should be eliminated. + foptimize_none<Special>(); +// { dg-final { scan-tree-dump-not "foptimize_none_special_failed *\\(\\)" "optimized" } } +} + +// { dg-final { scan-tree-dump-times "__builtin_snprintf" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-pure.C b/gcc/testsuite/g++.dg/ext/attr-pure.C new file mode 100644 index 0000000..af36a31 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-pure.C @@ -0,0 +1,69 @@ +/* PR c++/83871 - wrong code for attribute const and pure on distinct + template specializations + { dg-do compile } + { dg-options "-O -Wall" } */ + +int __attribute__ ((pure)) fpure_none (); +int fpure_none (); + +void test_pure_none_failed (); + +void func_pure_none () +{ + int i0 = fpure_none (); + int i1 = fpure_none (); + if (i0 != i1) + test_pure_none_failed (); + + // { dg-final { scan-tree-dump-not "test_pure_none_failed" "optimized" } } +} + + +int fnone_pure (); +int __attribute__ ((pure)) fnone_pure (); + +void test_none_pure_failed (); + +void func_none_pure () +{ + int i0 = fnone_pure (); + int i1 = fnone_pure (); + if (i0 != i1) + test_none_pure_failed (); + + // { dg-final { scan-tree-dump-not "test_none_pure_failed" "optimized" } } +} + + +template <class T> +int __attribute__ ((pure)) fpure_none (T); + +template <class T> +int fpure_none (T); + +void template_pure_none () +{ + int i0 = fpure_none<int> (0); + int i1 = fpure_none<int> (0); + if (i0 != i1) + test_pure_none_failed (); + + // { dg-final { scan-tree-dump-not "test_pure_none_failed" "optimized" } } +} + + +template <class T> +int fnone_pure (T); + +template <class T> +int __attribute__ ((pure)) fnone_pure (T); + +void test_fnone_pure () +{ + int i0 = fnone_pure<int> (0); + int i1 = fnone_pure<int> (0); + if (i0 != i1) + test_none_pure_failed (); + + // { dg-final { scan-tree-dump-not "test_none_pure_failed" "optimized" } } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C new file mode 100644 index 0000000..f75f32e --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C @@ -0,0 +1,42 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((returns_nonnull)) +g (); + +template <> +void* +g<int>(); + +extern void g_void_returns_nonnull (); +extern void g_int_may_return_null (); + +void test_returns_nonnull () +{ + void *p = g<void>(); + if (!p) + g_void_returns_nonnull (); + + (void)&p; +} + +void test_may_return_null () +{ + void *p = g<int>(); + if (!p) + g_int_may_return_null (); + + (void)&p; +} + + +// Verify that the call to g_void_returns_nonnull() is eliminated but +// the call to g_int_may_return_null() is retained. +// { dg-final { scan-tree-dump-not "g_void_returns_nonnull" "optimized" } } +// { dg-final { scan-tree-dump-times "g_int_may_return_null" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-warning.C b/gcc/testsuite/g++.dg/ext/attr-warning.C new file mode 100644 index 0000000..8369bac --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-warning.C @@ -0,0 +1,49 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute warning from a primary template declared with +// it. +// { dg-do compile } +// { dg-options "-Wall -fdump-tree-optimized" } + +struct Special; + +// Primary has no attributes here. +template <class T> +void fwarn_primary (); + +// Uses of the primary template, including declarations of its +// specializations, should not be diagnosed until after it has +// been redeclared with attribute warning. +template <> +void fwarn_primary<Special> (); + +void use_primary_before_warning () +{ + // Verify that uses of the primary are not diagnosed. + fwarn_primary<char>(); + fwarn_primary<short>(); +} + +// Redeclare the primary with attribute warning. +template <class T> +void __attribute__ ((warning ("primary"))) +fwarn_primary (); + +// Attribute warning is special in that it only warns for functions +// that are actually used, not those that are only declared. +template <> +void fwarn_primary<double> (); + +void use_primary_after_warning () +{ + // Verify that uses of the redeclared primary are diagnosed. + fwarn_primary<int>(); // { dg-warning "primary" } + fwarn_primary<long>(); // { dg-warning "primary" } +} + +void use_special () +{ + // Verify that the use of the specializatoin is not diagnosed. + fwarn_primary<Special>(); +}
On 02/26/2018 11:19 PM, Martin Sebor wrote: >>> While reviewing other related bugs I noticed 83502. This patch >>> doesn't fix the first test case in the bug (attribute noinline >>> vs always_inline). Somehow those are still copied from >>> the primary to the specialization and can cause conflicts. >> >> Hmm, that's odd. Why is that? > > Because duplicate_decl calls diagnose_mismatched_attributes() > on the NEWDECL and OLDDECL. (Attribute optimize would do the > same thing.) I was trying to keep the fix small but it makes > sense to take care of this as well so I have in this revision. > >>> It does fix the second test case but with the noreturn change >>> it would issue a bogus -Wmissing-attributes warning for the >>> explicit specialization below. Adding the warn_unused_result >>> attribute to it would then make GCC complain about a conflict >>> between the added attribute and noreturn, while removing it >>> would lead to worse code. >>> >>> template <class T> >>> int __attribute__ ((warn_unused_result)) f (T) { return 0; } >>> >>> template <> >>> int __attribute__ ((noreturn)) f<int> (int) { throw 0; } >>> >>> void fi () { f (0); } >> >> I continue to disagree with this use of attribute noreturn. >>> + /* Merge the function-has-side-effects bit. */ >>> + if (TREE_THIS_VOLATILE (newdecl)) >>> + TREE_THIS_VOLATILE (olddecl) = 1; >>> + >>> + if (merge_attr) >> >> TREE_THIS_VOLATILE means attribute noreturn, not whether the function >> has side-effects; it should be handled in the blocks controlled by >> merge_attr. > > Whoops. That was a silly goof. I must have misread the comment > above the macro definition. I also didn't have a test for it (or > some of the other changes I've made) so I didn't see the problem. > > Attached is an enhanced version of the patch that handles (and > tests) more of the commonly used attributes. I'm not sure why > in the merge_attr block I have to merge TREE_THIS_VOLATILE and > TREE_NOTHROW back and forth but not also READONLY, PURE, or > MALLOC, but without it tests fail. Because the memcpy from newdecl to olddecl at the end of duplicate_decls explicitly excludes the tree_common section. I don't know why that is, it certainly complicates the logic. That choice seems to predate the C++ front end. > PS Would it be possible to add a new macro with "noreturn" in > the name to make it more intuitive? (And ditto perhaps also > for TREE_READONLY for "const" functions, though for whatever > reason that seems easier to decipher. I know you're all used > to it but it's far from intuitive.) Sounds good. > PPS Duplicate_decls is over 1,400 lines long. If there is more > work to do here in stage 1 (I suspect there might be), would you > mind if I broke it up into two or more, say one for functions, > another for types, or whatever grouping makes most sense to make > it easier to follow? Sure, there's plenty of scope for cleaning up duplicate_decls. :) > + else > { > if (DECL_DECLARED_INLINE_P (newdecl)) > DECL_DISREGARD_INLINE_LIMITS (newdecl) = true; This looks like it will mean setting DECL_DISREGARD_INLINE_LIMITS on all inline template specializations. I think you want to drop these two lines. OK with that change. Jason
On Mon, Feb 26, 2018 at 09:19:56PM -0700, Martin Sebor wrote: > + /* Put together a list of the black listed attributes that the primary > + template is declared with that the specialization is not, in case > + it's not apparent from the most recent declaration of the primary. */ > + unsigned nattrs = 0; > + std::string str; > + > + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) > + { > + for (unsigned j = 0; j != 2; ++j) > + { > + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) > + continue; > + > + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) > + { > + if (lookup_attribute (blacklist[i], spec_attrs[k])) > + break; > + > + if (str.size ()) > + str += ", "; > + str += "%<"; > + str += blacklist[i]; > + str += "%>"; > + ++nattrs; > + } > + } > + } > + > + if (!nattrs) > + return; > + > + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attributes, > + "explicit specialization %q#D may be missing attributes", > + spec)) > + { > + if (nattrs > 1) > + str = G_("missing primary template attributes ") + str; > + else > + str = G_("missing primary template attribute ") + str; > + > + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); This is broken for multiple reasons: 1) it should be inform_n rather than inform 2) you really can't do what you're doing for translations; G_(...) marks the string for translations, but what actually is translated is not that string, but rather what is passed to inform, i.e. str.c_str (), so it will be likely never translated 3) as others have mentioned, the #include <string> you are doing is wrong 4) I don't see justification to use std::string here What you IMHO should use instead is use pretty_printer str; instead, and the pp_* APIs to add stuff in there, including pp_begin_quote (&str, pp_show_color (global_dc->printer)) and pp_end_quote (&str, pp_show_color (global_dc->printer)) when you want to add what %< or %> expand to, and finally inform_n (DECL_SOURCE_LOCATION (tmpl), nattrs, "missing primary template attribute %s", "missing primary template attributes %s", pp_formatted_text (&str)); That way it should be properly translatable. Jakub
On 02/27/2018 04:44 PM, Jakub Jelinek wrote: > On Mon, Feb 26, 2018 at 09:19:56PM -0700, Martin Sebor wrote: >> + /* Put together a list of the black listed attributes that the primary >> + template is declared with that the specialization is not, in case >> + it's not apparent from the most recent declaration of the primary. */ >> + unsigned nattrs = 0; >> + std::string str; >> + >> + for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) >> + { >> + for (unsigned j = 0; j != 2; ++j) >> + { >> + if (!lookup_attribute (blacklist[i], tmpl_attrs[j])) >> + continue; >> + >> + for (unsigned k = 0; k != 1 + !!spec_attrs[1]; ++k) >> + { >> + if (lookup_attribute (blacklist[i], spec_attrs[k])) >> + break; >> + >> + if (str.size ()) >> + str += ", "; >> + str += "%<"; >> + str += blacklist[i]; >> + str += "%>"; >> + ++nattrs; >> + } >> + } >> + } >> + >> + if (!nattrs) >> + return; >> + >> + if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attributes, >> + "explicit specialization %q#D may be missing attributes", >> + spec)) >> + { >> + if (nattrs > 1) >> + str = G_("missing primary template attributes ") + str; >> + else >> + str = G_("missing primary template attribute ") + str; >> + >> + inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); > > This is broken for multiple reasons: > 1) it should be inform_n rather than inform > 2) you really can't do what you're doing for translations; > G_(...) marks the string for translations, but what actually is > translated is not that string, but rather what is passed to inform, > i.e. str.c_str (), so it will be likely never translated > 3) as others have mentioned, the #include <string> you are doing is > wrong > 4) I don't see justification to use std::string here > > What you IMHO should use instead is use > pretty_printer str; > instead, and the pp_* APIs to add stuff in there, including > pp_begin_quote (&str, pp_show_color (global_dc->printer)) > and > pp_end_quote (&str, pp_show_color (global_dc->printer)) > when you want to add what %< or %> expand to, > and finally > inform_n (DECL_SOURCE_LOCATION (tmpl), nattrs, > "missing primary template attribute %s", > "missing primary template attributes %s", > pp_formatted_text (&str)); > That way it should be properly translatable. Using inform_n() would not be correct here. What's being translated is one of exactly two forms: singular and plural. It doesn't matter how many things the plural form refers to because the number doesn't appear in the message. Let's ask Google to translate the message above to a language with more than two plural forms, such as Czech: there are missing attributes: https://translate.google.com/?tl=cs#auto/cs/there%20are%20missing%20attributes vs there are 5 missing attributes: https://translate.google.com/?tl=cs#auto/cs/there%20are%205%20missing%20attributes Only the first form is correct when the exact number isn't mentioned. There are many places in the C++ front-end where a string enclosed in G_() is assigned to a pointer and later used in a diagnostic call. Is there something different about the usage I introduced that makes it unsuitable for translation? std::string is used in a number of places in GCC. Why does using it here need any special justification? Using the pretty printer as you suggest also sounds complicated to me and so prone to error but I will defer to Jason's opinion to decide if any changes are necessary. Martin PS What I do think would be helpful (and what I'd like to look into adding in stage 1) is a directive to format attribute lists. But I'm not sure the directive will help with cases like this one.
On Mon, Feb 26, 2018 at 09:19:56PM -0700, Martin Sebor wrote: > PR c++/83871 > PR c++/83503 > * g++.dg/Wmissing-attributes.C: New test. > * g++.dg/ext/attr-const-pure.C: New test. > * g++.dg/ext/attr-const.C: New test. > * g++.dg/ext/attr-deprecated-2.C: New test. > * g++.dg/ext/attr-malloc-2.C: New test. > * g++.dg/ext/attr-malloc.C: New test. > * g++.dg/ext/attr-noinline-2.C: New test. > * g++.dg/ext/attr-noinline.C: New test. > * g++.dg/ext/attr-nonnull.C: New test. > * g++.dg/ext/attr-noreturn-2.C: New test. > * g++.dg/ext/attr-noreturn.C: New test. > * g++.dg/ext/attr-nothrow-2.C: New test. > * g++.dg/ext/attr-nothrow.C: New test. > * g++.dg/ext/attr-optimize.C: New test. > * g++.dg/ext/attr-pure.C: New test. > * g++.dg/ext/attr-returns-nonnull.C: New test. > * g++.dg/ext/attr-warning.C: New test. I'm getting: +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_const_none_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_const_none_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_none_const_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_none_const_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++14 scan-tree-dump-not optimized "test_const_none_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++14 scan-tree-dump-not optimized "test_const_none_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++14 scan-tree-dump-not optimized "test_none_const_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++14 scan-tree-dump-not optimized "test_none_const_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++98 scan-tree-dump-not optimized "test_const_none_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++98 scan-tree-dump-not optimized "test_const_none_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++98 scan-tree-dump-not optimized "test_none_const_failed" +UNRESOLVED: g++.dg/ext/attr-const.C -std=gnu++98 scan-tree-dump-not optimized "test_none_const_failed" +UNRESOLVED: g++.dg/ext/attr-noinline-2.C -std=gnu++11 scan-tree-dump-not optimized" "fnoinline_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline-2.C -std=gnu++14 scan-tree-dump-not optimized" "fnoinline_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline-2.C -std=gnu++98 scan-tree-dump-not optimized" "fnoinline_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline.C -std=gnu++11 scan-tree-dump-not optimized" "fnoinline_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline.C -std=gnu++11 scan-tree-dump-not optimized" "fnone_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline.C -std=gnu++14 scan-tree-dump-not optimized" "fnoinline_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline.C -std=gnu++14 scan-tree-dump-not optimized" "fnone_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline.C -std=gnu++98 scan-tree-dump-not optimized" "fnoinline_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noinline.C -std=gnu++98 scan-tree-dump-not optimized" "fnone_always_inline<Special> *\\\\(\\\\);" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++11 scan-tree-dump-not optimized "fnone_noreturn_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++11 scan-tree-dump-not optimized "fnoreturn_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++11 scan-tree-dump-not optimized "fnoreturn_none_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++14 scan-tree-dump-not optimized "fnone_noreturn_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++14 scan-tree-dump-not optimized "fnoreturn_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++14 scan-tree-dump-not optimized "fnoreturn_none_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++98 scan-tree-dump-not optimized "fnone_noreturn_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++98 scan-tree-dump-not optimized "fnoreturn_failed" +UNRESOLVED: g++.dg/ext/attr-noreturn-2.C -std=gnu++98 scan-tree-dump-not optimized "fnoreturn_none_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++11 scan-tree-dump-not optimized "test_none_pure_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++11 scan-tree-dump-not optimized "test_none_pure_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++11 scan-tree-dump-not optimized "test_pure_none_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++11 scan-tree-dump-not optimized "test_pure_none_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++14 scan-tree-dump-not optimized "test_none_pure_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++14 scan-tree-dump-not optimized "test_none_pure_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++14 scan-tree-dump-not optimized "test_pure_none_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++14 scan-tree-dump-not optimized "test_pure_none_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++98 scan-tree-dump-not optimized "test_none_pure_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++98 scan-tree-dump-not optimized "test_none_pure_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++98 scan-tree-dump-not optimized "test_pure_none_failed" +UNRESOLVED: g++.dg/ext/attr-pure.C -std=gnu++98 scan-tree-dump-not optimized "test_pure_none_failed" after this commit. The following patch fixes the easy part of it, redundant -fdump-tree-* options when there is nothing that scans the file, syntax errors in scan-tree-dump-not which cause the UNRESOLVED messages as well -fdump-tree-eh in dg-options while scan-tree* scanning optimized dump instead. Tested on x86_64-linux with: make check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} dg.exp='attr*.C'" committed to trunk as obvious. 2018-02-28 Jakub Jelinek <jakub@redhat.com> PR c++/83871 PR c++/83503 * g++.dg/ext/attr-warning.C: Remove -fdump-tree-optimized from dg-options. * g++.dg/ext/attr-nonnull.C: Likewise. * g++.dg/ext/attr-noinline.C: Fix syntax in scan-tree-dump-not directives. * g++.dg/ext/attr-noinline-2.C: Likewise. * g++.dg/ext/attr-noreturn-2.C: Use -fdump-tree-optimized instead of -fdump-tree-eh in dg-options. --- gcc/testsuite/g++.dg/ext/attr-warning.C.jj 2018-02-28 09:55:57.234897906 +0100 +++ gcc/testsuite/g++.dg/ext/attr-warning.C 2018-02-28 10:21:43.195091237 +0100 @@ -4,7 +4,7 @@ // "inherit" attribute warning from a primary template declared with // it. // { dg-do compile } -// { dg-options "-Wall -fdump-tree-optimized" } +// { dg-options "-Wall" } struct Special; --- gcc/testsuite/g++.dg/ext/attr-nonnull.C.jj 2018-02-28 09:55:57.236897905 +0100 +++ gcc/testsuite/g++.dg/ext/attr-nonnull.C 2018-02-28 10:24:41.550989113 +0100 @@ -4,7 +4,7 @@ // does not "inherit" attribute nonnull from an argument declared with // one in the primary template. // { dg-do compile } -// { dg-options "-O -Wall -fdump-tree-optimized" } +// { dg-options "-O -Wall" } template <class T> void __attribute__ ((nonnull (1))) --- gcc/testsuite/g++.dg/ext/attr-noinline.C.jj 2018-02-28 09:55:57.234897906 +0100 +++ gcc/testsuite/g++.dg/ext/attr-noinline.C 2018-02-28 10:26:44.132918927 +0100 @@ -94,7 +94,7 @@ void test_keep_primary_3 (void) void test_elim_special_3 (void) { fnone_always_inline<Special>(); -// { dg-final { scan-tree-dump-not "fnone_always_inline<Special> *\\(\\);" optimized" } } +// { dg-final { scan-tree-dump-not "fnone_always_inline<Special> *\\(\\);" "optimized" } } } @@ -124,5 +124,5 @@ void test_keep_primary_4 (void) void test_elim_special_4 (void) { fnoinline_always_inline<Special>(); -// { dg-final { scan-tree-dump-not "fnoinline_always_inline<Special> *\\(\\);" optimized" } } +// { dg-final { scan-tree-dump-not "fnoinline_always_inline<Special> *\\(\\);" "optimized" } } } --- gcc/testsuite/g++.dg/ext/attr-noinline-2.C.jj 2018-02-28 09:55:57.235897906 +0100 +++ gcc/testsuite/g++.dg/ext/attr-noinline-2.C 2018-02-28 10:27:01.777908823 +0100 @@ -69,5 +69,5 @@ void test_elim_special_2 (void) { // Should be inlined. fnoinline_always_inline<Special>(); -// { dg-final { scan-tree-dump-not "fnoinline_always_inline<Special> *\\(\\);" optimized" } } +// { dg-final { scan-tree-dump-not "fnoinline_always_inline<Special> *\\(\\);" "optimized" } } } --- gcc/testsuite/g++.dg/ext/attr-noreturn-2.C.jj 2018-02-28 09:55:57.235897906 +0100 +++ gcc/testsuite/g++.dg/ext/attr-noreturn-2.C 2018-02-28 10:16:03.767285586 +0100 @@ -3,7 +3,7 @@ Test to verify that attributes noreturn on multiple declarations of the same function are merged. { dg-do compile } - { dg-options "-O -fdump-tree-eh" } */ + { dg-options "-O -fdump-tree-optimized" } */ int __attribute__ ((noreturn)) fnoreturn (); Jakub
On Mon, Feb 26, 2018 at 09:19:56PM -0700, Martin Sebor wrote: > PR c++/83871 > PR c++/83503 > * g++.dg/ext/attr-const.C: New test. > * g++.dg/ext/attr-pure.C: New test. I've tried to fix these 2 tests with following patch, without -fdump-tree-optimized all the scan-tree-dump* tests are UNRESOLVED, and when you use the same function for multiple-subtests all you get is the same failures reported multiple times, but without knowing which of them failed. Unfortunately, even with this patch there are failures: FAIL: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_func_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_template_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++14 scan-tree-dump-not optimized "test_func_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++14 scan-tree-dump-not optimized "test_template_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++17 -fconcepts scan-tree-dump-not optimized "test_func_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++17 -fconcepts scan-tree-dump-not optimized "test_template_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++17 scan-tree-dump-not optimized "test_func_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++17 scan-tree-dump-not optimized "test_template_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++2a scan-tree-dump-not optimized "test_func_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++2a scan-tree-dump-not optimized "test_template_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++98 scan-tree-dump-not optimized "test_func_none_const_failed" FAIL: g++.dg/ext/attr-const.C -std=gnu++98 scan-tree-dump-not optimized "test_template_none_const_failed" FAIL: g++.dg/ext/attr-pure.C -std=gnu++11 scan-tree-dump-not optimized "test_template_none_pure_failed" FAIL: g++.dg/ext/attr-pure.C -std=gnu++14 scan-tree-dump-not optimized "test_template_none_pure_failed" FAIL: g++.dg/ext/attr-pure.C -std=gnu++17 -fconcepts scan-tree-dump-not optimized "test_template_none_pure_failed" FAIL: g++.dg/ext/attr-pure.C -std=gnu++17 scan-tree-dump-not optimized "test_template_none_pure_failed" FAIL: g++.dg/ext/attr-pure.C -std=gnu++2a scan-tree-dump-not optimized "test_template_none_pure_failed" FAIL: g++.dg/ext/attr-pure.C -std=gnu++98 scan-tree-dump-not optimized "test_template_none_pure_failed" so if the test is right, then something is still broken on the C++ FE side. I'll defer debugging this to you. --- gcc/testsuite/g++.dg/ext/attr-const.C.jj 2018-02-28 09:55:57.235897906 +0100 +++ gcc/testsuite/g++.dg/ext/attr-const.C 2018-02-28 10:37:09.775438593 +0100 @@ -1,37 +1,37 @@ /* PR c++/83871 - wrong code for attribute const and pure on distinct template specializations { dg-do compile } - { dg-options "-O -Wall" } */ + { dg-options "-O -Wall -fdump-tree-optimized" } */ int __attribute__ ((const)) fconst_none (); int fconst_none (); -void test_const_none_failed (); +void test_func_const_none_failed (); void func_const_none () { int i0 = fconst_none (); int i1 = fconst_none (); if (i0 != i1) - test_const_none_failed (); + test_func_const_none_failed (); - // { dg-final { scan-tree-dump-not "test_const_none_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_func_const_none_failed" "optimized" } } } int fnone_const (); int __attribute__ ((const)) fnone_const (); -void test_none_const_failed (); +void test_func_none_const_failed (); void func_none_const () { int i0 = fnone_const (); int i1 = fnone_const (); if (i0 != i1) - test_none_const_failed (); + test_func_none_const_failed (); - // { dg-final { scan-tree-dump-not "test_none_const_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_func_none_const_failed" "optimized" } } } @@ -41,14 +41,16 @@ int __attribute__ ((const)) fconst_none template <class T> int fconst_none (T); +void test_template_const_none_failed (); + void template_const_none () { int i0 = fconst_none<int> (0); int i1 = fconst_none<int> (0); if (i0 != i1) - test_const_none_failed (); + test_template_const_none_failed (); - // { dg-final { scan-tree-dump-not "test_const_none_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_template_const_none_failed" "optimized" } } } @@ -58,12 +60,14 @@ int fnone_const (T); template <class T> int __attribute__ ((const)) fnone_const (T); +void test_template_none_const_failed (); + void test_fnone_const () { int i0 = fnone_const<int> (0); int i1 = fnone_const<int> (0); if (i0 != i1) - test_none_const_failed (); + test_template_none_const_failed (); - // { dg-final { scan-tree-dump-not "test_none_const_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_template_none_const_failed" "optimized" } } } --- gcc/testsuite/g++.dg/ext/attr-pure.C.jj 2018-02-28 09:55:57.236897905 +0100 +++ gcc/testsuite/g++.dg/ext/attr-pure.C 2018-02-28 10:35:39.864510362 +0100 @@ -1,37 +1,37 @@ /* PR c++/83871 - wrong code for attribute const and pure on distinct template specializations { dg-do compile } - { dg-options "-O -Wall" } */ + { dg-options "-O -Wall -fdump-tree-optimized" } */ int __attribute__ ((pure)) fpure_none (); int fpure_none (); -void test_pure_none_failed (); +void test_func_pure_none_failed (); void func_pure_none () { int i0 = fpure_none (); int i1 = fpure_none (); if (i0 != i1) - test_pure_none_failed (); + test_func_pure_none_failed (); - // { dg-final { scan-tree-dump-not "test_pure_none_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_func_pure_none_failed" "optimized" } } } int fnone_pure (); int __attribute__ ((pure)) fnone_pure (); -void test_none_pure_failed (); +void test_func_none_pure_failed (); void func_none_pure () { int i0 = fnone_pure (); int i1 = fnone_pure (); if (i0 != i1) - test_none_pure_failed (); + test_func_none_pure_failed (); - // { dg-final { scan-tree-dump-not "test_none_pure_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_func_none_pure_failed" "optimized" } } } @@ -41,14 +41,16 @@ int __attribute__ ((pure)) fpure_none (T template <class T> int fpure_none (T); +void test_template_pure_none_failed (); + void template_pure_none () { int i0 = fpure_none<int> (0); int i1 = fpure_none<int> (0); if (i0 != i1) - test_pure_none_failed (); + test_template_pure_none_failed (); - // { dg-final { scan-tree-dump-not "test_pure_none_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_template_pure_none_failed" "optimized" } } } @@ -58,12 +60,14 @@ int fnone_pure (T); template <class T> int __attribute__ ((pure)) fnone_pure (T); +void test_template_none_pure_failed (); + void test_fnone_pure () { int i0 = fnone_pure<int> (0); int i1 = fnone_pure<int> (0); if (i0 != i1) - test_none_pure_failed (); + test_template_none_pure_failed (); - // { dg-final { scan-tree-dump-not "test_none_pure_failed" "optimized" } } + // { dg-final { scan-tree-dump-not "test_template_none_pure_failed" "optimized" } } } Jakub
On 02/28/2018 02:51 AM, Jakub Jelinek wrote: > On Mon, Feb 26, 2018 at 09:19:56PM -0700, Martin Sebor wrote: >> PR c++/83871 >> PR c++/83503 >> * g++.dg/ext/attr-const.C: New test. >> * g++.dg/ext/attr-pure.C: New test. > > I've tried to fix these 2 tests with following patch, without > -fdump-tree-optimized all the scan-tree-dump* tests are UNRESOLVED, > and when you use the same function for multiple-subtests all you get > is the same failures reported multiple times, but without knowing which > of them failed. > > Unfortunately, even with this patch there are failures: > FAIL: g++.dg/ext/attr-const.C -std=gnu++11 scan-tree-dump-not optimized "test_func_none_const_failed" ... > so if the test is right, then something is still broken on the C++ FE side. > > I'll defer debugging this to you. Thanks for fixing up the unresolved tests. I have a script that checks test results against a baseline for unexpected failures but it doesn't look for differences in UNRESOLVED so I didn't see them as problems. I'll have to remember to enhance it to do that. With the unresolved problems fixed, the remaining failures are due to two problems: 1) A regression in the merging of attributes introduced by my patch. I wondered about why attributes const and pure were being handled differently than the others here: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01479.html Jason explained that parts of the structure were being copied via memcpy. Since I thought my tests were passing I took that to mean that the bits were being copied. Sigh. 2) A pre-existing bug where the C++ front end doesn't apply an attribute to a declaration of a function template previously declared without one, as in: template <class T> int f (); template <class T> int __attribute__ ((const)) f (); The root cause might be the same as the one behind bug 84294 - missing warning for ignored attribute on a function template declaration. Let me fix the first one and see what it would take to handle the second as well. Martin
On Tue, Feb 27, 2018 at 7:52 PM, Martin Sebor <msebor@gmail.com> wrote: > There are many places in the C++ front-end where a string > enclosed in G_() is assigned to a pointer and later used > in a diagnostic call. Is there something different about > the usage I introduced that makes it unsuitable for > translation? The difference is that you append to the string before it is used. Looks like the _ macro might work better in this case? Jason
PR c++/83871 - wrong code for attribute const and pure on distinct template specializations PR c++/83503 - [8 Regression] bogus -Wattributes for const and pure on function template specialization gcc/cp/ChangeLog: PR c++/83871 PR c++/83503 * decl.c (duplicate_decls): Avoid applying primary function template's attributes to its explicit specializations. gcc/testsuite/ChangeLog: PR c++/83871 PR c++/83503 * g++.dg/ext/attr-const-pure.C: New test. * g++.dg/ext/attr-malloc.C: New test. * g++.dg/ext/attr-nonnull.C: New test. * g++.dg/ext/attr-nothrow.C: New test. * g++.dg/ext/attr-returns-nonnull.C: New test. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 244a3ef..21c3051 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -1971,10 +1971,21 @@ next_arg:; DECL_ORIGINAL_TYPE (newdecl) = DECL_ORIGINAL_TYPE (olddecl); } - /* Copy all the DECL_... slots specified in the new decl - except for any that we copy here from the old type. */ - DECL_ATTRIBUTES (newdecl) - = (*targetm.merge_decl_attributes) (olddecl, newdecl); + /* True to merge attributes between the declarations, false to + set OLDDECL's attributes to those of NEWDECL (for template + explicit specializations that specify their own attributes + independent of those specified for the primary template). */ + const bool merge_attr = (TREE_CODE (newdecl) != FUNCTION_DECL + || !DECL_TEMPLATE_SPECIALIZATION (newdecl) + || DECL_TEMPLATE_SPECIALIZATION (olddecl)); + + /* Copy all the DECL_... slots specified in the new decl except for + any that we copy here from the old type. */ + if (merge_attr) + DECL_ATTRIBUTES (newdecl) + = (*targetm.merge_decl_attributes) (olddecl, newdecl); + else + DECL_ATTRIBUTES (olddecl) = DECL_ATTRIBUTES (newdecl); if (DECL_DECLARES_FUNCTION_P (olddecl) && DECL_DECLARES_FUNCTION_P (newdecl)) { @@ -2148,6 +2159,17 @@ next_arg:; && !tx_safe_fn_type_p (TREE_TYPE (newdecl))) newtype = tx_unsafe_fn_variant (newtype); + if (!merge_attr) + { + /* Remove the two function attributes that are, in fact, + treated as (quasi) type attributes. */ + tree attrs = TYPE_ATTRIBUTES (newtype); + tree newattrs = remove_attribute ("nonnull", attrs); + newattrs = remove_attribute ("returns_nonnull", attrs); + if (newattrs != attrs) + TYPE_ATTRIBUTES (newtype) = newattrs; + } + TREE_TYPE (newdecl) = TREE_TYPE (olddecl) = newtype; if (TREE_CODE (newdecl) == FUNCTION_DECL) @@ -2167,13 +2189,24 @@ next_arg:; && !(processing_template_decl && uses_template_parms (newdecl))) layout_decl (newdecl, 0); - /* Merge the type qualifiers. */ - if (TREE_READONLY (newdecl)) - TREE_READONLY (olddecl) = 1; if (TREE_THIS_VOLATILE (newdecl)) TREE_THIS_VOLATILE (olddecl) = 1; - if (TREE_NOTHROW (newdecl)) - TREE_NOTHROW (olddecl) = 1; + + if (merge_attr) + { + /* Merge the type qualifiers. */ + if (TREE_READONLY (newdecl)) + TREE_READONLY (olddecl) = 1; + if (TREE_NOTHROW (newdecl)) + TREE_NOTHROW (olddecl) = 1; + } + else + { + /* Set the bits that correspond to the const and nothrow + function attributes. */ + TREE_READONLY (olddecl) = TREE_READONLY (newdecl); + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + } /* Merge deprecatedness. */ if (TREE_DEPRECATED (newdecl)) @@ -2212,13 +2245,22 @@ next_arg:; |= DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (olddecl); DECL_NO_LIMIT_STACK (newdecl) |= DECL_NO_LIMIT_STACK (olddecl); TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); - TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); - DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); - DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); DECL_LOOPING_CONST_OR_PURE_P (newdecl) |= DECL_LOOPING_CONST_OR_PURE_P (olddecl); + if (merge_attr) + { + TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl); + DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); + DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); + } + else + { + TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl); + DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl); + DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl); + } /* Keep the old RTL. */ COPY_DECL_RTL (olddecl, newdecl); } @@ -2340,17 +2382,19 @@ next_arg:; { tree parm; - /* Merge parameter attributes. */ + /* Merge or assign parameter attributes. */ tree oldarg, newarg; - for (oldarg = DECL_ARGUMENTS(olddecl), - newarg = DECL_ARGUMENTS(newdecl); - oldarg && newarg; - oldarg = DECL_CHAIN(oldarg), newarg = DECL_CHAIN(newarg)) { - DECL_ATTRIBUTES (newarg) - = (*targetm.merge_decl_attributes) (oldarg, newarg); - DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); - } - + for (oldarg = DECL_ARGUMENTS (olddecl), + newarg = DECL_ARGUMENTS (newdecl); + oldarg && newarg; + oldarg = DECL_CHAIN (oldarg), newarg = DECL_CHAIN (newarg)) + { + if (merge_attr) + DECL_ATTRIBUTES (newarg) + = (*targetm.merge_decl_attributes) (oldarg, newarg); + DECL_ATTRIBUTES (oldarg) = DECL_ATTRIBUTES (newarg); + } + if (DECL_TEMPLATE_INSTANTIATION (olddecl) && !DECL_TEMPLATE_INSTANTIATION (newdecl)) { diff --git a/gcc/testsuite/g++.dg/ext/attr-const-pure.C b/gcc/testsuite/g++.dg/ext/attr-const-pure.C new file mode 100644 index 0000000..6afef6d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-const-pure.C @@ -0,0 +1,103 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +int __attribute__ ((pure)) +f (T); + +template <> +int __attribute__ ((const)) f<int> (int); // { dg-bogus "ignoring attribute .const." } + + +template <class T> +int __attribute__ ((const)) +g (T); + +template <> +int __attribute__ ((pure)) g<int> (int); // { dg-bogus "ignoring attribute .const." } + +template <class T> +int __attribute__ ((const)) +h (T); + +template <class T> +int __attribute__ ((pure)) +h (const T*); + +template <> +int h<int> (int); + +template <> +int h<int*> (int*); + +int global; + +extern void h_primary_elim (); +extern void h_cstptr_elim (); +extern void h_cstptr_keep (); +extern void h_int_keep (); +extern void h_intptr_keep (); + +void call_primary_elim (double x) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. + int i0 = h (x); + int i1 = h (x); + + if (i0 != i1) // must be folded into false + h_primary_elim (); // must be eliminated +} + +void call_cstptr_elim (const void *p) +{ + // Only the first call to h(x) must be emitted, the second one + // is expected to be eliminated. This verifies that h<const + // void*>*() is treated as const in this context. + int i0 = h (p); + int i1 = h (p); + + if (i0 != i1) // must be folded into false + h_cstptr_elim (); // must be eliminated +} + +void call_cstptr_keep (const void *p) +{ + // Because of the store to the global, both calls to h(p) must + // be emitted. This verifies that h<const void*>*() is not + // treated as const. + int i0 = h (p); + global = 123; + int i1 = h (p); + + if (i0 != i1) // must not be folded + h_cstptr_keep (); // must be emitted +} + +void call_int_keep (int i) +{ + // Both calls to h(i) must be emitted. + int i0 = h (i); + int i1 = h (i); + + if (i0 != i1) // must not be folded + h_int_keep (); // must be emitted +} + +void call_intptr_keep (int *ip) +{ + // Both calls to h(ip) must be emitted. + int i0 = h (ip); + int i1 = h (ip); + + if (i0 != i1) // must not be folded + h_intptr_keep (); // must be emitted +} + +// { dg-final { scan-tree-dump-not "h_primary_elim" "optimized" } } +// { dg-final { scan-tree-dump-not "h_cstptr_elim" "optimized" } } +// { dg-final { scan-tree-dump-times "h_cstptr_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_int_keep" 1 "optimized" } } +// { dg-final { scan-tree-dump-times "h_intptr_keep" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-malloc.C b/gcc/testsuite/g++.dg/ext/attr-malloc.C new file mode 100644 index 0000000..a680677 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-malloc.C @@ -0,0 +1,45 @@ +// Bug c++/83503 - bogus -Wattributes for const and pure on function template +// specialization +// Test to verify that an explicit template specifialization does not +// "inherit" attribute malloc from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((malloc)) +f (unsigned); + +template <> +void* +f<int>(unsigned); + +static char a[8]; + +void f_void_malloc (); +void f_int_not_malloc (); + +void fv (void) +{ + void *p = f<void>(1); + if (!p) + return; + + if (p == a) // must be false + f_void_malloc (); // should be eliminated +} + + +void fi (void) +{ + void *p = f<int>(1); + if (!p) + return; + + if (p == a) // can be true + f_int_not_malloc (); // must not be eliminated +} + +// Verify that the call to f_void_not_malloc() is eliminated but +// the call to f_int_not_malloc() is retained. +// { dg-final { scan-tree-dump-not "f_void_malloc" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_not_malloc" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-nonnull.C new file mode 100644 index 0000000..16b152f --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nonnull.C @@ -0,0 +1,31 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nonnull (1))) +f (T*, T*, T*); + +template <> +void +f<int>(int*, int*, int*); + +template <> +void __attribute__ ((nonnull (3))) +f<float>(float*, float*, float*); + + +void test_nonnull (void) +{ + f<void>(0, 0, 0); // { dg-warning "null argument where non-null required \\\(argument 1\\\)" } + + f<int>(0, 0, 0); // { dg-bogus "null argument" } + + f<float>(0, 0, 0); + // { dg-bogus "null argument where non-null required \\\(argument 1\\\)" "" { target *-*-* } .-1 } + // { dg-warning "null argument where non-null required \\\(argument 3\\\)" "" { target *-*-* } .-2 } +} diff --git a/gcc/testsuite/g++.dg/ext/attr-nothrow.C b/gcc/testsuite/g++.dg/ext/attr-nothrow.C new file mode 100644 index 0000000..ae6b674 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-nothrow.C @@ -0,0 +1,47 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit template specifialization does not +// "inherit" attribute nothrow from a primary template declared with one. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void __attribute__ ((nothrow)) +f (); + +template <> +void +f<int>(); + +void f_void_nothrow (); +void f_int_maythrow (); + +void fv (void) +{ + try + { + f<void>(); + } + catch (...) // cannot be be reached + { + f_void_nothrow (); // should be eliminated + } +} + + +void fi (void) +{ + try + { + f<int>(); + } + catch (...) // may be reached + { + f_int_maythrow (); // must not be eliminated + } +} + +// Verify that the call to f_void_nothrow() is eliminated but +// the call to f_int_maythrow() is retained. +// { dg-final { scan-tree-dump-not "f_void_nothrow" "optimized" } } +// { dg-final { scan-tree-dump-times "f_int_maythrow" 1 "optimized" } } diff --git a/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C new file mode 100644 index 0000000..f75f32e --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-returns-nonnull.C @@ -0,0 +1,42 @@ +// Bug c++/83871 - wrong code due to attributes on distinct template +// specializations +// Test to verify that an explicit function template specifialization +// does not "inherit" attribute nonnull from an argument declared with +// one in the primary template. +// { dg-do compile } +// { dg-options "-O -Wall -fdump-tree-optimized" } + +template <class T> +void* __attribute__ ((returns_nonnull)) +g (); + +template <> +void* +g<int>(); + +extern void g_void_returns_nonnull (); +extern void g_int_may_return_null (); + +void test_returns_nonnull () +{ + void *p = g<void>(); + if (!p) + g_void_returns_nonnull (); + + (void)&p; +} + +void test_may_return_null () +{ + void *p = g<int>(); + if (!p) + g_int_may_return_null (); + + (void)&p; +} + + +// Verify that the call to g_void_returns_nonnull() is eliminated but +// the call to g_int_may_return_null() is retained. +// { dg-final { scan-tree-dump-not "g_void_returns_nonnull" "optimized" } } +// { dg-final { scan-tree-dump-times "g_int_may_return_null" 1 "optimized" } }
To resolve the underlying root cause of the P1 bug c++/83503 - bogus -Wattributes for const and pure on function template specialization, that we discussed last week, I've taken a stab at making the change to avoid applying primary template's attributes to its explicit specializations. (The bug tracking the underlying root cause is 83871 - wrong code for attribute const and pure on distinct template specializations). The attached patch is what I have so far. It's incomplete and not all the tests pass for a couple of reasons: 1) it only handles function templates (not class templates), and I have no tests for those yet, 2) it isn't effective for the nonnull and returns_nonnull attributes because they are treated as type attributes, 3) the change to deal with attributes on function arguments may be unnecessary (I couldn't come up with any that would be propagated from the primary -- are there some?). Before I proceed with it I first want to make sure that it should be fixed for GCC 8, that duplicate_decls is the right place for these changes, and that (2) should be fixed by treating those and other such attributes by applying them to function decls. We've talked about (2) in the past (bug 71463) but this seems like an opportunity to revisit it and (hopefully) make a change to treat these the same as all other function attributes rather than type attributes. Besides fixing the wrong code bugs and false positives it will also make them more intuitive to use. There have been a number of bug reports from users confused by the -Wignored-attributes warnings caused by this unusual handling. Thanks Martin