diff mbox series

merge attributes from function template redeclarations (PR 84294)

Message ID 9790b00b-c571-a1ea-6dfd-338a3125fa0d@gmail.com
State New
Headers show
Series merge attributes from function template redeclarations (PR 84294) | expand

Commit Message

Martin Sebor Feb. 28, 2018, 10:47 p.m. UTC 
Attached is a patch for the failure to merge a subset of attributes
specified on redeclarations of a function template with those of
the first declaration (const, malloc, pure, noinline, noreturn,
and nothrow).  This was uncovered this morning while debugging
failures in tests I added just yesterday and although it's not
a regression I figured since I introduced the test failures and
the fix appears easy it might as well be taken care of now.

The test I added for this exposed a subtle bug in -Wreturn-type
where the warning triggers on a function template instantiated
on a noreturn function even though the middle end successfully
eliminates statements after the noreturn call.  This is not
a regression either so to avoid mission creep I just xfailed
the test and opened bug 84621 to remind us to fix it in
the future.

Bootstrap passes, a full regression test suite run is underway.

Martin

Comments

Jason Merrill March 1, 2018, 7:28 p.m. UTC | #1 
On Wed, Feb 28, 2018 at 5:47 PM, Martin Sebor <msebor@gmail.com> wrote:
> Attached is a patch for the failure to merge a subset of attributes
> specified on redeclarations of a function template with those of
> the first declaration (const, malloc, pure, noinline, noreturn,
> and nothrow).  This was uncovered this morning while debugging
> failures in tests I added just yesterday and although it's not
> a regression I figured since I introduced the test failures and
> the fix appears easy it might as well be taken care of now.
>
> The test I added for this exposed a subtle bug in -Wreturn-type
> where the warning triggers on a function template instantiated
> on a noreturn function even though the middle end successfully
> eliminates statements after the noreturn call.  This is not
> a regression either so to avoid mission creep I just xfailed
> the test and opened bug 84621 to remind us to fix it in
> the future.

OK.  Incidentally, looking over duplicate_decls again, I noticed:

          /* [temp.expl.spec/14] We don't inline explicit
specialization
             just because the primary template says so.  */

          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
            {
              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);
            }

The upper block here seems like dead code; here we're dealing with an
explicit specialization, so merge_attr should always be false.  Let's
drop the if block.

Jason
Martin Sebor March 2, 2018, 12:18 a.m. UTC | #2 
On 03/01/2018 12:28 PM, Jason Merrill wrote:
> On Wed, Feb 28, 2018 at 5:47 PM, Martin Sebor <msebor@gmail.com> wrote:
>> Attached is a patch for the failure to merge a subset of attributes
>> specified on redeclarations of a function template with those of
>> the first declaration (const, malloc, pure, noinline, noreturn,
>> and nothrow).  This was uncovered this morning while debugging
>> failures in tests I added just yesterday and although it's not
>> a regression I figured since I introduced the test failures and
>> the fix appears easy it might as well be taken care of now.
>>
>> The test I added for this exposed a subtle bug in -Wreturn-type
>> where the warning triggers on a function template instantiated
>> on a noreturn function even though the middle end successfully
>> eliminates statements after the noreturn call.  This is not
>> a regression either so to avoid mission creep I just xfailed
>> the test and opened bug 84621 to remind us to fix it in
>> the future.
>
> OK.  Incidentally, looking over duplicate_decls again, I noticed:
>
>           /* [temp.expl.spec/14] We don't inline explicit
> specialization
>              just because the primary template says so.  */
>
>           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
>             {
>               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);
>             }
>
> The upper block here seems like dead code; here we're dealing with an
> explicit specialization, so merge_attr should always be false.  Let's
> drop the if block.
>

Done in r258121.  I also added an assert there just to be sure.

Martin
diff mbox series

Patch

PR c++/84294 - attributes on a function template redeclaration silently discarded

gcc/cp/ChangeLog:

	PR c++/84294
	* decl.c (check_redeclaration_no_default_args): Merge attributes
	specified on redeclarations of the same function template.

gcc/testsuite/ChangeLog:

	PR c++/84294
	* g++.dg/ext/attr-const.C: Remove xfail.
	* g++.dg/ext/attr-malloc-3.C: New test.
	* g++.dg/ext/attr-noinline-3.C: New test.
	* g++.dg/ext/attr-noreturn-3.C: New test.
	* g++.dg/ext/attr-nothrow-3.C: New test.
	* g++.dg/ext/attr-pure.C: Remove xfail.

Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 258077)
+++ gcc/cp/decl.c	(working copy)
@@ -1355,6 +1355,26 @@  check_redeclaration_no_default_args (tree decl)
       }
 }
 
+/* Merge tree bits that correspond to attributes noreturn, nothrow,
+   const,  malloc, and pure from NEWDECL with those of OLDDECL.  */
+
+static void
+merge_attribute_bits (tree newdecl, tree olddecl)
+{
+  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);
+  TREE_READONLY (olddecl) |= TREE_READONLY (newdecl);
+  DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl);
+  DECL_IS_MALLOC (olddecl) |= DECL_IS_MALLOC (newdecl);
+  DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl);
+  DECL_PURE_P (olddecl) |= DECL_PURE_P (newdecl);
+  DECL_UNINLINABLE (newdecl) |= DECL_UNINLINABLE (olddecl);
+  DECL_UNINLINABLE (olddecl) |= DECL_UNINLINABLE (newdecl);
+}
+
 #define GNU_INLINE_P(fn) (DECL_DECLARED_INLINE_P (fn)			\
 			  && lookup_attribute ("gnu_inline",		\
 					       DECL_ATTRIBUTES (fn)))
@@ -2048,6 +2068,8 @@  next_arg:;
 	      DECL_DISREGARD_INLINE_LIMITS (old_result)
 	        |= DECL_DISREGARD_INLINE_LIMITS (new_result);
 	      check_redeclaration_exception_specification (newdecl, olddecl);
+
+	      merge_attribute_bits (new_result, old_result);
 	    }
 	}
 
@@ -2228,18 +2250,7 @@  next_arg:;
 	    |= 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);
-	      TREE_READONLY (olddecl) |= TREE_READONLY (newdecl);
-	      DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl);
-	      DECL_IS_MALLOC (olddecl) |= DECL_IS_MALLOC (newdecl);
-	      DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl);
-	      DECL_PURE_P (olddecl) |= DECL_PURE_P (newdecl);
-	    }
+	    merge_attribute_bits (newdecl, olddecl);
 	  else
 	    {
 	      /* Merge the noreturn bit.  */
Index: gcc/testsuite/g++.dg/ext/attr-const.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-const.C	(revision 258077)
+++ gcc/testsuite/g++.dg/ext/attr-const.C	(working copy)
@@ -68,6 +68,5 @@  void test_fnone_const ()
   if (i0 != i1)
     templ_none_const_failed ();
 
-  // The following fails (most likely) due to bug 84294.
-  // { dg-final { scan-tree-dump-not "templ_none_const_failed" "optimized" { xfail *-*-* } } }
+  // { dg-final { scan-tree-dump-not "templ_none_const_failed" "optimized" } }
 }
Index: gcc/testsuite/g++.dg/ext/attr-malloc-3.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-malloc-3.C	(revision 258077)
+++ gcc/testsuite/g++.dg/ext/attr-malloc-3.C	(working copy)
@@ -91,7 +91,6 @@  void test_templ_none_malloc (void)
   if (p == a)                       // must be false
     templ_none_malloc_failed ();    // should be eliminated
 
-  // The following fails (most likely) due to bug 84294.
   // Verify that the call to templ_none_malloc_failed() is eliminated.
-  // { dg-final { scan-tree-dump-not "templ_none_malloc_failed" "optimized" { xfail *-*-* } } }
+  // { dg-final { scan-tree-dump-not "templ_none_malloc_failed" "optimized" } }
 }
Index: gcc/testsuite/g++.dg/ext/attr-noinline-3.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-noinline-3.C	(nonexistent)
+++ gcc/testsuite/g++.dg/ext/attr-noinline-3.C	(working copy)
@@ -0,0 +1,45 @@ 
+/*  PR c++/84294 - attributes on a function template redeclaration silently
+    discarded
+    { dg-do compile }
+    { dg-options "-O -fdump-tree-optimized" } */
+
+template <void test ()>
+void test_func ()
+{
+  test ();
+}
+
+int x;
+
+void __attribute__ ((noinline)) func_noinline_none ();
+void func_noinline_none () { x = __LINE__; }
+
+template void test_func<func_noinline_none>();
+// { dg-final { scan-tree-dump-times "func_noinline_none *\\(\\);" 1 "optimized" } }
+
+
+void func_none_noinline ();
+void  __attribute__ ((noinline)) func_none_noinline () { x = __LINE__; }
+
+template void test_func<func_none_noinline>();
+// { dg-final { scan-tree-dump-times "func_none_noinline *\\(\\);" 1 "optimized" } }
+
+
+template <class>
+void __attribute__ ((noinline)) templ_noinline_none () { x = __LINE__; }
+
+template <class>
+void templa_noinline_none ();
+
+template void test_func<templ_noinline_none<int> >();
+// { dg-final { scan-tree-dump-times "templ_noinline_none<int> *\\(\\);" 1 "optimized" } }
+
+
+template <class>
+void templ_none_noinline ();
+
+template <class>
+void  __attribute__ ((noinline)) templ_none_noinline () { x = __LINE__; }
+
+template void test_func<templ_none_noinline<int> >();
+// { dg-final { scan-tree-dump-times "templ_none_noinline<int> *\\(\\);" 1 "optimized" } }
Index: gcc/testsuite/g++.dg/ext/attr-noreturn-3.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-noreturn-3.C	(nonexistent)
+++ gcc/testsuite/g++.dg/ext/attr-noreturn-3.C	(working copy)
@@ -0,0 +1,54 @@ 
+/*  PR c++/84294 - attributes on a function template redeclaration silently
+    discarded
+    { dg-do compile }
+    { dg-options "-O -fdump-tree-optimized" } */
+
+typedef void Func ();
+
+template <Func>
+void fail_func ();
+
+template <Func test>
+int test_func ()
+{
+  test ();
+
+  // Should be eliminated.
+  fail_func<test> ();
+
+  // Expect no -Wreturn type here despite the absence of a return
+  // statement in a non-void function.
+}   // { dg-bogus "\\\[-Wreturn-type]" "bug 84621" { xfail *-*-* } }
+
+void __attribute__ ((noreturn)) func_noreturn_none ();
+void func_noreturn_none ();
+
+template int test_func<func_noreturn_none>();
+
+
+void func_none_noreturn ();
+void  __attribute__ ((noreturn)) func_none_noreturn ();
+
+template int test_func<func_none_noreturn>();
+
+
+template <class>
+void __attribute__ ((noreturn)) templ_noreturn_none ();
+
+template <class>
+void templa_noreturn_none ();
+
+template int test_func<templ_noreturn_none<int> >();
+
+
+template <class>
+void templ_none_noreturn ();
+
+template <class>
+void  __attribute__ ((noreturn)) templ_none_noreturn ();
+
+template int test_func<templ_none_noreturn<int> >();
+
+
+// Verify that calls to fail_func() specializations have been eliminated.
+// { dg-final { scan-tree-dump-not "fail_func" "optimized" } }
Index: gcc/testsuite/g++.dg/ext/attr-nothrow-3.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-nothrow-3.C	(nonexistent)
+++ gcc/testsuite/g++.dg/ext/attr-nothrow-3.C	(working copy)
@@ -0,0 +1,60 @@ 
+/*  PR c++/84294 - attributes on a function template redeclaration silently
+    discarded
+    { dg-do compile }
+    { dg-options "-O -fdump-tree-eh -fdump-tree-optimized" } */
+
+typedef void Func ();
+
+template <Func>
+void fail_func () throw ();
+
+template <Func test>
+void test_func () throw ()
+{
+  try
+    {
+      test ();
+    }
+  catch (...)
+    {
+      // Should be eliminated.
+      fail_func<test> ();
+    }
+}
+
+void __attribute__ ((nothrow)) func_nothrow_none ();
+void func_nothrow_none ();
+
+template void test_func<func_nothrow_none>();
+
+
+void func_none_nothrow ();
+void  __attribute__ ((nothrow)) func_none_nothrow ();
+
+template void test_func<func_none_nothrow>();
+
+
+template <class>
+void __attribute__ ((nothrow)) templ_nothrow_none ();
+
+template <class>
+void templa_nothrow_none ();
+
+template void test_func<templ_nothrow_none<int> >();
+
+
+template <class>
+void templ_none_nothrow ();
+
+template <class>
+void  __attribute__ ((nothrow)) templ_none_nothrow ();
+
+template void test_func<templ_none_nothrow<int> >();
+
+
+// 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" } }
+
+// Verify that calls to fail_func() specializations have been eliminated.
+// { dg-final { scan-tree-dump-not "fail_func" "optimized" } }
Index: gcc/testsuite/g++.dg/ext/attr-pure.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-pure.C	(revision 258077)
+++ gcc/testsuite/g++.dg/ext/attr-pure.C	(working copy)
@@ -69,6 +69,5 @@  void test_fnone_const ()
   if (i0 != i1)
     templ_none_const_failed ();
 
-  // The following fails (most likely) due to bug 84294.
-  // { dg-final { scan-tree-dump-not "templ_none_const_failed" "optimized" { xfail *-*-* } } }
+  // { dg-final { scan-tree-dump-not "templ_none_const_failed" "optimized" } }
 }