This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)


On 11/02/2016 02:11 AM, Bernd Edlinger wrote:
On 11/01/16 19:15, Bernd Edlinger wrote:
On 11/01/16 18:11, Jason Merrill wrote:
On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
On 11/01/16 16:20, Jason Merrill wrote:
On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
I'm not even sure we need a new warning.  Can we combine this warning
with the block that currently follows?

After 20 years of not having a warning on that,
an implicitly enabled warning would at least break lots of bogus
test cases.

Would it, though?  Which test cases still break with the current patch?

Less than before, but there are still at least a few of them.

I can make a list and send it tomorrow.

FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
errors)
FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
errors)
FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
-flto-partition=1to1 -fno-use-linker-plugin
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
-flto-partition=none -fuse-linker-plugin
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
-fuse-linker-plugin -fno-fat-lto-objects
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
-flto-partition=1to1 -fno-use-linker-plugin
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
-flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
-fuse-linker-plugin
FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess errors)
FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess errors)
FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess errors)
FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess errors)
FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess errors)
FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess errors)
FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
errors)
FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
errors)
FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess errors)
FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess errors)
FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess errors)
FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess errors)
FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess errors)
FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess errors)


The lto test case does emit the warning when assembling, but
it still produces an executable and even executes it.

Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
and g++.old-deja/g++.other/vbase5.C are execution tests.

So I was wrong to assume these were all compile-only tests.

I think that list should be fixable, if we decide to enable
the warning by default.

Yes, either by fixing the prototypes or disabling the warning.

If we remove the DECL_ANTICIPATED check, I see some failures in
builtin* tests due to missing extern "C".  That seems appropriate at
file scope, but I'm not sure it's right for namespace std.

Interesting, what have you done?

The attached. But now that you've found that the existing warning deals with people doing silly things like redeclaring __builtin_* I guess let's leave it alone, and add the warning to the DECL_ANTICIPATED block the way you proposed.

Jason

commit d93d421435f8c38b2019526c7645a59e79a92cc5
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Nov 1 17:16:12 2016 -0400

    rem-ant

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index ecf4d14..963087d 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1485,10 +1485,6 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
 	}
       else if (!types_match)
 	{
-	  /* Avoid warnings redeclaring built-ins which have not been
-	     explicitly declared.  */
-	  if (DECL_ANTICIPATED (olddecl))
-	    {
 	  /* Deal with fileptr_type_node.  FILE type is not known
 	     at the time we create the builtins.  */
 	  tree t1, t2;
@@ -1521,8 +1517,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
 	      }
 	    else if (! same_type_p (TREE_VALUE (t1), TREE_VALUE (t2)))
 	      break;
-	    }
-	  else if ((DECL_EXTERN_C_P (newdecl)
+
+	  if ((DECL_EXTERN_C_P (newdecl)
 	       && DECL_EXTERN_C_P (olddecl))
 	      || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
 			    TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
@@ -1540,7 +1536,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
                          : G_("shadowing library function %q#D"), olddecl);
 	    }
 	  else
-	    /* Discard the old built-in function.  */
+	    /* Not a duplicate.  */
 	    return NULL_TREE;
 
 	  /* Replace the old RTL to avoid problems with inlining.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]