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: [RFA] PR c++/41020


On Fri, Oct 23, 2009 at 04:51:18PM -0700, Jason Merrill wrote:
> Does it also compile without the patch?  What I was trying to get at  
> was, are we doing something else to make block-scope externs work that  
> we should also do for friend functions?  I don't know the answer.

Thanks for clarifying your question. I didn't actually got what you wanted
at first.

So I looked into what happens when compiling the following program:

~=~
extern "C" int fork (void);

void
foo ()
{
  extern int fork (void);
  fork ();
}

extern "C"
int
fork (void)
{
  return 0;
}
~=~

It compiles fine, but looking at the generated assembly, I can see this in
the body of the foo function:

 call    _Z4forkv

So we are calling a version of fork that has C++ linkage, where I think we
should be calling the version of fork with C linkage.

In pushdecl_maybe_friend, we detect the extern function re-declaration
we but we fail to link the re-declaration to the previous extern "C"
declaration because decls_match fails at:

 if (t && t != error_mark_node)
        {
          if (different_binding_level)
            {
              if (decls_match (x, t))
                /* The standard only says that the local extern
                   inherits linkage from the previous decl; in
                   particular, default args are not shared.  Add
                   the decl into a hash table to make sure only
                   the previous decl in this case is seen by the
                   middle end.  */
                {

That linking should have taken place in the body of the if (decls_match (x, t))).
And of course decls_match fails for the same reason it is failing in
duplicate_decls for the friend case.

So what happens instead is that the re-declared decl is pushed locally in
foo as if it was a new declaration with c++ linkage.

> In any case, a friend declaration should not match a builtin  
> declaration.  It shouldn't give an error, either, it should declare a  
> different function.

>From what you said in http://gcc.gnu.org/ml/gcc-patches/2009-10/msg01501.html
could we say that in this case (block-scope extern or friend declarations
coming after explicit builtin declarations) we should match the
explicit extern "C" declaration of the builtin and thus reference it ?

If we can say that then I think the current handling of block-scope extern
declarations should be changed so that we can match explicitely declared
builtin functions.

So I have updated the patch to make decls_match use DECL_IS_BUILTIN instead of
DECL_BUILT_IN. From what I understand, I think the former doesn't catch
explicit declarations of builtin functions.

I have added more regression tests to the patch as well.

Tested against trunk on x86-64-unknown-linux-gnu.

Thanks.

commit ebae280b67752cf5d2a537e5157cb3c9a618a6ed
Author: Dodji Seketeli <dodji@redhat.com>
Date:   Fri Oct 23 21:15:12 2009 +0200

    Fix PR c++/41020
    
    gcc/cp/ChangeLog:
    
    	PR c++/41020
    	* decl.c (decls_match): Use DECL_IS_BUILTIN instead of
    	DECL_BUILT_IN.
    
    gcc/testsuite/ChangeLog:
    	PR c++/41020
    	* g++.dg/lookup/extern-c-redecl2.C: New test.
    	* g++.dg/lookup/extern-c-redecl3.C: Likewise.
    	* g++.dg/lookup/extern-c-redecl4.C: Likewise.
    	* g++.dg/lookup/extern-c-redecl5.C: Likewise.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5eb389f..c772ca5 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -935,7 +935,7 @@ decls_match (tree newdecl, tree olddecl)
 #ifdef NO_IMPLICIT_EXTERN_C
       /* A new declaration doesn't match a built-in one unless it
 	 is also extern "C".  */
-      if (DECL_BUILT_IN (olddecl)
+      if (DECL_IS_BUILTIN (olddecl)
 	  && DECL_EXTERN_C_P (olddecl) && !DECL_EXTERN_C_P (newdecl))
 	return 0;
 #endif
diff --git a/gcc/testsuite/g++.dg/lookup/extern-c-redecl2.C b/gcc/testsuite/g++.dg/lookup/extern-c-redecl2.C
new file mode 100644
index 0000000..055148f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/extern-c-redecl2.C
@@ -0,0 +1,21 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin PR c++/41020
+// { dg-do compile }
+
+extern "C"
+{
+  int fork (void);
+}
+
+class frok
+{
+  int this_errno;
+  friend int fork (void);
+};
+
+extern "C" int
+fork (void)
+{
+  frok grouped;
+  return grouped.this_errno;
+}
diff --git a/gcc/testsuite/g++.dg/lookup/extern-c-redecl3.C b/gcc/testsuite/g++.dg/lookup/extern-c-redecl3.C
new file mode 100644
index 0000000..00ff4a9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/extern-c-redecl3.C
@@ -0,0 +1,22 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin: PR c++/41020
+// { dg-do compile }
+// { dg-final { scan-assembler-not "call\[\t \]+_Z4forkv" } }
+// { dg-final { scan-assembler "call\[\t \]+fork" } }
+
+extern "C" int fork (void);
+
+void
+foo ()
+{
+  extern int fork (void);
+  fork ();
+}
+
+extern "C"
+int
+fork (void)
+{
+  return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C b/gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C
new file mode 100644
index 0000000..9dfa54d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C
@@ -0,0 +1,19 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin: PR c++/41020
+
+// Avoid the "-ansi -pedantic" option
+// { dg-options "" }
+// { dg-do compile }
+// { dg-final { scan-assembler "call\[\t \]+_Z4forkv" } }
+
+class frok
+{
+  int this_errno;
+  friend int fork (void);
+};
+
+void
+foo ()
+{
+  fork ();
+}
diff --git a/gcc/testsuite/g++.dg/lookup/extern-c-redecl5.C b/gcc/testsuite/g++.dg/lookup/extern-c-redecl5.C
new file mode 100644
index 0000000..031059f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/extern-c-redecl5.C
@@ -0,0 +1,18 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin: PR c++/41020
+// { dg-do compile }
+
+
+class frok
+{
+  int this_errno;
+  friend int fork (void); // { dg-error "previous declaration .*?C++. linkage" }
+};
+
+extern "C" int
+fork (void) // { dg-error "conflicts with new declaration .*?C. linkage" }}
+{
+  frok grouped;
+  return grouped.this_errno;
+}
+

-- 
Dodji Seketeli
Red Hat


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