[RFC PATCH] avoid applying attributes to explicit specializations (PR 83871)

[Martin Sebor <msebor at gmail dot com>]

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

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" } }