public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC PATCH] avoid applying attributes to explicit specializations (PR 83871)
@ 2018-02-05  0:07 Martin Sebor
  2018-02-05 21:53 ` Jason Merrill
  2018-02-05 23:44 ` Joseph Myers
  0 siblings, 2 replies; 34+ messages in thread
From: Martin Sebor @ 2018-02-05  0:07 UTC (permalink / raw)
  To: Jason Merrill, Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]

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

[-- Attachment #2: gcc-83871.diff --]
[-- Type: text/x-patch, Size: 13603 bytes --]

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

^ permalink raw reply	[flat|nested] 34+ messages in thread
* Re: [RFC PATCH] avoid applying attributes to explicit specializations (PR 83871)
@ 2018-02-27 23:21 David Edelsohn
  2018-02-27 23:30 ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: David Edelsohn @ 2018-02-27 23:21 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jason Merrill, GCC Patches

Martin,

This patch broke bootstrap.

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"

Nothing is allowed to be included before GCC config.h and system.h.
And you should not be including C++ header files directly.  If you
truly need <string>, the file should define INCLUDE_STRING (see
system.h).

Thanks, David

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2018-02-28 16:38 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-05  0:07 [RFC PATCH] avoid applying attributes to explicit specializations (PR 83871) Martin Sebor
2018-02-05 21:53 ` Jason Merrill
2018-02-07  4:01   ` Martin Sebor
2018-02-07 19:42     ` Jason Merrill
2018-02-08 21:52       ` Martin Sebor
2018-02-09 19:52         ` Jason Merrill
2018-02-09 23:58           ` Martin Sebor
2018-02-12 16:30             ` Jason Merrill
2018-02-12 16:59               ` Martin Sebor
2018-02-12 17:11                 ` Jason Merrill
2018-02-12 23:40                   ` Martin Sebor
2018-02-13 14:47                     ` Jason Merrill
2018-02-13 18:00                       ` Martin Sebor
2018-02-13 18:34                         ` Jason Merrill
2018-02-21 23:04                           ` Martin Sebor
2018-02-22 22:12                             ` Jason Merrill
2018-02-27  4:20                               ` Martin Sebor
2018-02-27 14:49                                 ` Jason Merrill
2018-02-27 23:49                                 ` Jakub Jelinek
2018-02-28  0:52                                   ` Martin Sebor
2018-02-28 14:49                                     ` [PATCH] Fix i18n in warn_spec_missing_attributes " Jakub Jelinek
2018-02-28 16:38                                       ` Jason Merrill
2018-02-28 16:10                                     ` [RFC PATCH] avoid applying attributes to explicit specializations " Jason Merrill
2018-02-28  9:49                                 ` Jakub Jelinek
2018-02-28  9:51                                 ` Jakub Jelinek
2018-02-28 16:05                                   ` Martin Sebor
2018-02-13 15:59                     ` Michael Matz
2018-02-13 17:34                       ` Martin Sebor
2018-02-05 23:44 ` Joseph Myers
2018-02-06  0:44   ` Martin Sebor
2018-02-06  0:55     ` Joseph Myers
2018-02-06  2:48       ` Martin Sebor
2018-02-27 23:21 David Edelsohn
2018-02-27 23:30 ` Martin Sebor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).