Bug 2316 - g++ fails to overload on language linkage
Summary: g++ fails to overload on language linkage
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 2.97
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: ABI, accepts-invalid, rejects-valid
: 28308 29038 29411 30062 40336 56683 (view as bug list)
Depends on:
Blocks: 29843
  Show dependency treegraph
 
Reported: 2001-03-18 17:46 UTC by Martin Sebor
Modified: 2014-06-27 12:37 UTC (History)
22 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-18 20:17:13


Attachments
remember linkage of a function type (998 bytes, patch)
2011-08-30 00:20 UTC, Marc Glisse
Details | Diff
remember linkage of a function type (2) (2.08 KB, patch)
2011-08-30 18:57 UTC, Marc Glisse
Details | Diff
remember linkage of a function type (3) (2.22 KB, patch)
2011-09-03 13:00 UTC, Marc Glisse
Details | Diff
remember linkage of a function type (4) (7.69 KB, patch)
2012-01-02 00:18 UTC, Marc Glisse
Details | Diff
remember linkage of a function type (5) (8.87 KB, patch)
2012-01-04 11:10 UTC, Marc Glisse
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2001-03-18 17:46:00 UTC
gcc incorrectly complains about an ambiguity between the
two overloads of foo() in the testcase below. This prevents
the correct implementation of std::bsearch() and
std::qsort() as per 25.4, p3 and 4, respectively.

$ gcc t.cpp
t.cpp: In function `int foo(int (*)())':
t.cpp:8: redefinition of `int foo(int (*)())'
t.cpp:2: `int foo(int (*)())' previously defined here

Release:
gcc version 2.97 20010122 (experimental)

How-To-Repeat:
extern "C" {
    int foo (int (*pf)()) { return pf (); }

    int bar () { return 0; }
}

extern "C++" {
    int foo (int (*pf)()) { return pf (); }

    int baz () { return 1; }
}

int main ()
{
    return !(0 == foo (bar) && 1 == foo (baz));
}
Comment 1 Nathan Sidwell 2001-04-25 08:31:57 UTC
State-Changed-From-To: open->feedback
State-Changed-Why: [25.4] Oh, yeah. Note, [13] does not talk about how to resolve
    language linkage and IIR you can't specify language
    linkage on a typedef pointer to function. Isn't there
    a defect report about this? Do you have some pointers
    to more info?
Comment 2 Nathan Sidwell 2001-04-25 15:31:58 UTC
From: nathan@gcc.gnu.org
To: gcc-gnats@gcc.gnu.org, nobody@gcc.gnu.org, sebor@roguewave.com
Cc:  
Subject: Re: c++/2316
Date: 25 Apr 2001 15:31:58 -0000

 Synopsis: gcc 2.97 fails to overload on language linkage
 
 State-Changed-From-To: open->feedback
 State-Changed-By: nathan
 State-Changed-When: Wed Apr 25 08:31:57 2001
 State-Changed-Why:
     [25.4] Oh, yeah. Note, [13] does not talk about how to resolve
     language linkage and IIR you can't specify language
     linkage on a typedef pointer to function. Isn't there
     a defect report about this? Do you have some pointers
     to more info?
 
 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view&pr=2316&database=gcc
Comment 3 Nathan Sidwell 2001-04-26 01:13:04 UTC
State-Changed-From-To: feedback->analyzed
State-Changed-Why: confirmed as a bug
Comment 4 Nathan Sidwell 2001-04-26 08:13:04 UTC
From: nathan@gcc.gnu.org
To: gcc-gnats@gcc.gnu.org, nobody@gcc.gnu.org, sebor@roguewave.com
Cc:  
Subject: Re: c++/2316
Date: 26 Apr 2001 08:13:04 -0000

 Synopsis: gcc 2.97 fails to overload on language linkage
 
 State-Changed-From-To: feedback->analyzed
 State-Changed-By: nathan
 State-Changed-When: Thu Apr 26 01:13:04 2001
 State-Changed-Why:
     confirmed as a bug
 
 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view&pr=2316&database=gcc

Comment 5 Wolfgang Bangerth 2003-05-05 19:59:59 UTC
From: Wolfgang Bangerth <bangerth@ices.utexas.edu>
To: gcc-gnats@gcc.gnu.org
Cc:  
Subject: Re: c++/2316
Date: Mon, 5 May 2003 19:59:59 -0500 (CDT)

 DR 4 may or may not be related to this.
 W.
Comment 6 Richard Biener 2005-09-09 12:56:32 UTC
EDG aggrees with gcc here.  Maybe not a bug.
Comment 7 Gabriel Dos Reis 2005-09-09 14:27:09 UTC
Subject: Re:  g++ fails to overload on language linkage

"rguenth at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| EDG aggrees with gcc here.  Maybe not a bug.

I don't know why I don't like that syllogism, especially when it is
applied to many PRs in row :-/

-- Gaby
Comment 8 Martin Sebor 2005-09-11 21:34:34 UTC
In reply to comment #6:
The vanilla EDG eccp 3.5 compiles the test case w/o an error:

$ eccp -v t.cpp && ./a.out; echo $?
Edison Design Group C/C++ Front End, version 3.5 (Nov  9 2004 20:00:33)
Copyright 1988-2004 Edison Design Group, Inc.

0

Intel C++ 9.0 (based on eccp 3.4.1) does reject the code, but that's most likely
for compatibility with gcc. I wouldn't rely on its behavior to determine whether
gcc is or isn't correct. (Note that HP aCC 6.0 which is also based on EDG eccp
accepts the code as expected.)

In reply to comment #4:
I don't think DR 4 is related since it's about internal linkage.

The test case is well-formed according to 7.5.1, p1, "Two function types with
different language linkages are distinct types even if they are otherwise
identical."

I.e., the first foo() can only take the address of an extern "C" function as an
argument (and not extern "C++") while the second foo() only an extern "C++"
function (and not extern "C"). Thus, each foo() is a distinct overload.
Comment 9 Andrew Pinski 2006-07-07 20:52:48 UTC
*** Bug 28308 has been marked as a duplicate of this bug. ***
Comment 10 Andrew Pinski 2006-09-12 16:55:08 UTC
*** Bug 29038 has been marked as a duplicate of this bug. ***
Comment 11 Andrew Pinski 2006-10-10 16:33:46 UTC
*** Bug 29411 has been marked as a duplicate of this bug. ***
Comment 12 Benjamin Kosnik 2006-10-10 20:07:43 UTC
adding ABI keyword as implementing this may change mangling for "C" functions.
Comment 13 Jonathan Wakely 2006-10-15 03:24:53 UTC
If this ever gets fixed (which I hope it does) then maybe it should depend on -std=c++98 so this continues to work by default, or it will break a lot of code that incorrectly passes extern "C++" functions to e.g. pthread_create and sigaction.  That's a lot of code.




Comment 14 pinskia@gmail.com 2006-10-15 03:29:00 UTC
Subject: Re:  g++ fails to overload on language linkage

On Sun, 2006-10-15 at 03:24 +0000, gcc-bugzilla at kayari dot org wrote:
> 
> ------- Comment #13 from gcc-bugzilla at kayari dot org  2006-10-15 03:24 -------
> If this ever gets fixed (which I hope it does) then maybe it should depend on
> -std=c++98 so this continues to work by default, or it will break a lot of code
> that incorrectly passes extern "C++" functions to e.g. pthread_create and
> sigaction.  That's a lot of code.
The problem is -std=c++98 vs no options should not produce different
code which can happen as shown in this bug already, that we have wrong
code.

-- Pinski

Comment 15 Marc Glisse 2006-12-04 15:54:15 UTC
*** Bug 30062 has been marked as a duplicate of this bug. ***
Comment 16 Andrew Pinski 2009-06-04 14:36:33 UTC
*** Bug 40336 has been marked as a duplicate of this bug. ***
Comment 17 Paolo Carlini 2009-06-04 14:49:56 UTC
Interestingly, the only current compiler I tried which actually accepts this (SunStudio) warns:

"2316.C", line 8: Warning: function int(int(*)()) overloads extern "C" int(extern "C" int(*)()) because of different language linkages.


Comment 18 Marc Glisse 2009-06-05 04:22:59 UTC
(In reply to comment #17)
I just checked, and the sunpro warning is overzealous. It is meant to catch cases where the programmer writes a declaration with one linkage and later provides a definition with some other linkage, which ends up as an overload and not a definition of the first function (this is a valid use, but may not be what the programmer intended). It is hard to warn about such wrong cases without also catching perfectly normal uses...

http://forums.sun.com/thread.jspa?threadID=5390323
Comment 19 Jason Merrill 2009-11-12 04:54:54 UTC
I was thinking that the ABI didn't distinguish between C and C++ function types, but I was wrong; it does specify a different mangling for extern "C" functions.  The problem is that currently G++ only tracks language linkage for declarations, not types.
Comment 20 Marc Glisse 2011-08-30 00:20:07 UTC
Created attachment 25134 [details]
remember linkage of a function type

This is an extremely basic patch, that probably misses many things (and we'd want to have conversions between extern"C" and regular functions not to break too much code), but it can be useful to play with (it got at least 4 examples right :-) although it ICEs (assert) on some other code...).

Obviously it has to be built with --disable-bootstrap since the gcc source code is far from extern"C"-clean, but it can also be used to help clean it ;-) Oh, and it won't build libstdc++ either, first because of functions in gthread that take &pthread_cancel and then because of the __stoa helper in ext/string_conversions.h (I didn't try to go further).

Function type trees apparently leave at least 2 trees unused in tree_type_non_common so I reused one (minval), but since I only want one bit, there are probably better ways to store it.
Comment 21 Jonathan Wakely 2011-08-30 09:12:23 UTC
Thanks, Marc! We can (and eventually should) fix anything in libstdc++ that incorrectly relies on this bug. Gthreads might be a little harder, but we could probably overload on language linkage if necessary, so the existing interfaces are preserved.
I'll try your patch and see what, if anything, can be changed safely in libstdc++ right away.
Comment 22 Marc Glisse 2011-08-30 09:32:24 UTC
(In reply to comment #21)
> I'll try your patch and see what, if anything, can be changed safely in
> libstdc++ right away.

Thanks :-)

Note that some things are painful to do right with extern "C". For instance, the __stoa helper takes as argument a pointer to a function with a type that depends on template arguments. As far as I can see, it is not possible to do that for extern "C" functions (I posted a question about that on clc++m yesterday), so that would mean rewriting this code differently. My guess is that making extern "C" functions usable would require a few DR, or extern "C" being part of the type will follow the example of "export" and be deprecated/removed (the second option is more likely, even if that's yet another change that hurts Oracle's compiler).
Comment 23 Jonathan Wakely 2011-08-30 10:46:51 UTC
(In reply to comment #22)
> Note that some things are painful to do right with extern "C". For instance,
> the __stoa helper takes as argument a pointer to a function with a type that
> depends on template arguments. As far as I can see, it is not possible to do
> that for extern "C" functions (I posted a question about that on clc++m
> yesterday), so that would mean rewriting this code differently.

I think you can do it with a alias-declaration in an extern "C" block:

extern "C" {
  template<typename T>
    using func_type = T (*)();
}

extern "C" void c() { }

template<typename T>
  void f( func_type<T> p ) { p(); }

int main()
{
  f(&c);
}

But yes, it's a bit of a hole in the type system that language linkage is part of the type but can only be specified in some contexts and can't be deduced by templates.  We could solve it with an alternative syntax for language linkage using attributes:

  void f( [[extern(C)]] void (*p)() );

could be equivalent to:

  extern "C" {  typedef void (*func_type)();  }
  void f( func_type p );

so we could say:

  template<typename T, typename U>
    void g( [[extern(C)]] T (*p)(U));


Without alias-declarations, the tedious way to fix __stoa would be write an extern "C++" forwarding function for each of the strtol, stroul, vsnprintf etc. functions we need, and pass that forwarding function to the helper function templates. That could be done with macros, and would still be simpler than re-implementing the __stoa error-checking logic in each one.  Anyway, that is probably straying off-topic for this PR.

> My guess is
> that making extern "C" functions usable would require a few DR, or extern "C"
> being part of the type will follow the example of "export" and be
> deprecated/removed (the second option is more likely, even if that's yet
> another change that hurts Oracle's compiler).

Clang++ and VC++ also "implement" this bug.  If it's fixed in G++ it would need to be controlled by a switch, maybe with the current behaviour as the default (but issuing a warning) for a couple of releases. As I said in c13, it could break a lot of code, plenty of people don't even know that they shouldn't be able to pass extern "C++" functions to C library APIs.
Comment 24 Jonathan Wakely 2011-08-30 10:49:27 UTC
(In reply to comment #23)
> I think you can do it with a alias-declaration in an extern "C" block:

But that might have only worked when I tested it because Clang++ doesn't overload on language linkage either, I'm not sure if it's actually valid.
Comment 25 Marc Glisse 2011-08-30 11:28:48 UTC
(In reply to comment #23)
> I think you can do it with a alias-declaration in an extern "C" block:
> 
> extern "C" {
>   template<typename T>
>     using func_type = T (*)();
> }
> 
> extern "C" void c() { }
> 
> template<typename T>
>   void f( func_type<T> p ) { p(); }
> 
> int main()
> {
>   f(&c);
> }
> 
> But yes, it's a bit of a hole in the type system that language linkage is part
> of the type but can only be specified in some contexts and can't be deduced by
> templates.

Yes, I thought about the alias, but it doesn't deduce the type. I really don't think your example works unless you call f<thetype>(&c) as aliases are not supposed to be deduced.

> We could solve it with an alternative syntax for language linkage
> using attributes:
> 
>   void f( [[extern(C)]] void (*p)() );

Is that where attributes go? That must be inconvenient for functions returning function pointers. Or does it have some kind of "scope"?

> could be equivalent to:
> 
>   extern "C" {  typedef void (*func_type)();  }
>   void f( func_type p );
> 
> so we could say:
> 
>   template<typename T, typename U>
>     void g( [[extern(C)]] T (*p)(U));

Yes, that would be nice.

My guess is that we are supposed to use extern "C" functions very rarely and only in very specific contexts where it doesn't matter so much if you can't do all the meta-programming stuff.

> Anyway, that is probably straying off-topic for this PR.

Finding reasons to ask for the removal of this feature from the next standard is kind of relevant ;-)

> Clang++ and VC++ also "implement" this bug.  If it's fixed in G++ it would need
> to be controlled by a switch, maybe with the current behaviour as the default
> (but issuing a warning) for a couple of releases. As I said in c13, it could
> break a lot of code, plenty of people don't even know that they shouldn't be
> able to pass extern "C++" functions to C library APIs.

Oracle's compiler implements it as different types, which already breaks a bit of code (but it also means that a number of large projects have been fixed for it), but it still allows many kinds of conversions between them, so most code only gives warnings.
Comment 26 Jonathan Wakely 2011-08-30 12:17:47 UTC
(In reply to comment #25)
> (In reply to comment #23)
> 
> > We could solve it with an alternative syntax for language linkage
> > using attributes:
> > 
> >   void f( [[extern(C)]] void (*p)() );
> 
> Is that where attributes go? That must be inconvenient for functions returning
> function pointers. Or does it have some kind of "scope"?

I probably have the syntax wrong, I wanted that attribute to apply to the parameter p, but for a function poiner maybe it should be:

void f( void (*p)() [[extern(C)]] );

And for a function pointer return type my attempt would be

void (* ugly(int)[[extern(C++)]] )() [[extern(C)]] ;

i.e. 

extern "C" {  typedef void (*c_func)() }

extern "C++" {  c_func ugly(int); }


Maybe.


> My guess is that we are supposed to use extern "C" functions very rarely and
> only in very specific contexts where it doesn't matter so much if you can't do
> all the meta-programming stuff.

I did suggest to the POSIX C++ WG that there should be extern "C++" overloads of pthread_create, pthread_atfork etc. just as we have overloads of qsort and bsearch taking pointers to extern "C++" functions, which would allow existing code that relies on this bug to just work without changes.  I was going to submit a proposal along those lines, but that WG seemed to fizzle out and I never finished the paper.  I probably still have it on an old hard drive on a shelf somewhere.
Comment 27 Jonathan Wakely 2011-08-30 12:19:23 UTC
this is DR13 btw
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#13
I don't know if that's been reconsidered now we have attributes
Comment 28 Marc Glisse 2011-08-30 12:34:20 UTC
(In reply to comment #27)
> this is DR13 btw
> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#13
> I don't know if that's been reconsidered now we have attributes

Gah, I looked for it, and once again forgot to look outside of the "active" list :-(

Yes, this is exactly this DR, thanks for digging it up.
Comment 29 Marc Glisse 2011-08-30 18:57:16 UTC
Created attachment 25140 [details]
remember linkage of a function type (2)

New version that works with typedefs (I was forgetting extern "C" in the canonical type...). The patch also includes a workaround for __stoa. There seems to still be an issue with argument deduction.

For some reason, -fpermissive already allows all conversions :-)
Comment 30 Marc Glisse 2011-08-30 20:32:57 UTC
(In reply to comment #29)
> New version that works with typedefs (I was forgetting extern "C" in the
> canonical type...). The patch also includes a workaround for __stoa. There
> seems to still be an issue with argument deduction.

Also need to fix strip_typedefs (I am not uploading a new patch right now):

--- gcc/cp/tree.c	(revision 178336)
+++ gcc/cp/tree.c	(working copy)
@@ -1151,8 +1151,8 @@
 	  }
 	else
 	  {
-	    result = build_function_type (type,
-					  arg_types);
+	    result = build_function_type2 (type, arg_types,
+			    TYPE_MINVAL (t) != 0);
 	    result = apply_memfn_quals (result, type_memfn_quals (t));
 	  }
Comment 31 Marc Glisse 2011-08-31 13:40:28 UTC
(In reply to comment #25)
> (In reply to comment #23)
> > I think you can do it with a alias-declaration in an extern "C" block:
> > 
> > extern "C" {
> >   template<typename T>
> >     using func_type = T (*)();
> > }
> > 
> > extern "C" void c() { }
> > 
> > template<typename T>
> >   void f( func_type<T> p ) { p(); }
> > 
> > int main()
> > {
> >   f(&c);
> > }
> > 
> > But yes, it's a bit of a hole in the type system that language linkage is part
> > of the type but can only be specified in some contexts and can't be deduced by
> > templates.
> 
> Yes, I thought about the alias, but it doesn't deduce the type. I really don't
> think your example works unless you call f<thetype>(&c) as aliases are not
> supposed to be deduced.

Sorry for that comment, I was completely wrong because of a gross misunderstanding of template aliases, your solution is great (as soon as gcc has template aliases ;-).

http://groups.google.com/group/comp.lang.c++.moderated/browse_thread/thread/43df1e789d3b44fe
Comment 32 Marc Glisse 2011-09-03 13:00:20 UTC
Created attachment 25181 [details]
remember linkage of a function type (3)

This version of the patch, with -fpermissive and without -Werror, bootstraps gcc without requiring any patching. -fpermissive does most of the work of allowing any conversion, I only had to add some code so "&operator new[]" (overloaded) knew which conversion to pick.

Obviously the patch needs completing (I am certainly missing many things), cleaning up, and additional work to allow conversions without -fpermissive, and only those that differ only by extern "C" somewhere. In other words it still needs to be (re)written ;-)

But I think it is very encouraging that it compiles gcc (including libstdc++) without any patching (but lots of warnings), which means this bug might be fixed without breaking too much user code (we can be more lax than sunpro, which rejects gcc sources in 2 places). And I don't think it introduces too much ambiguity (overloading on linkage still works on a few tests).
Comment 33 Marc Glisse 2011-09-04 11:03:41 UTC
And if you don't like errors saying that X can't be converted to X, you'll need something like the below. I don't think I'll go much further anytime soon (if someone else wants a go, that'd be great...).

--- cp/error.c  (revision 178506)
+++ cp/error.c  (working copy)
@@ -805,6 +805,8 @@
          pp_cxx_cv_qualifier_seq (cxx_pp, class_of_this_parm (t));
        else
          pp_cxx_cv_qualifier_seq (cxx_pp, t);
+       if (TREE_CODE (t) == FUNCTION_TYPE && TYPE_MINVAL (t))
+         pp_string (cxx_pp, " extern \"C\"");
        dump_exception_spec (TYPE_RAISES_EXCEPTIONS (t), flags);
        dump_type_suffix (TREE_TYPE (t), flags);
        break;
Comment 34 Marc Glisse 2011-09-15 16:53:33 UTC
I posted a related demangler patch on gcc-patches a couple weeks ago, let me just link it from here so it doesn't get lost:
http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00231.html
Comment 35 Marc Glisse 2012-01-02 00:18:19 UTC
Created attachment 26214 [details]
remember linkage of a function type (4)

I updated the patch to work with alias templates. I also hacked the library enough to complete a --disable-bootstrap --enable-languages=c,c++ build. It works well enough to play with it and see how buggy every code out there is... I didn't implement conversions, -fpermissive or C casts are good enough as long as this is just a toy.
Comment 36 Jonathan Wakely 2012-01-02 01:04:14 UTC
The library should overload qsort, then the libitm/clone.cc change wouldn't be needed
Comment 37 Andrew Pinski 2012-01-02 02:30:17 UTC
I think the change to tree.c should not be done as it is middle-end code.  That should be in the C++ front-end specific code instead.  That is the middle-end should not care about the difference between extern "C" function types and extern "C++" ones.
Comment 38 Marc Glisse 2012-01-02 10:39:35 UTC
Thanks for the comments.

(In reply to comment #36)
> The library should overload qsort, then the libitm/clone.cc change wouldn't be
> needed

Indeed, that was the meaning of my comment "provide the true qsort prototype instead", I'll do that next time.


(In reply to comment #37)
> I think the change to tree.c should not be done as it is middle-end code.  That
> should be in the C++ front-end specific code instead.  That is the middle-end
> should not care about the difference between extern "C" function types and
> extern "C++" ones.

That makes sense. I am surprised to see that I only use the new function once outside of the cp/ directory, and I don't even know whether that is really necessary. On the other hand, there may be other places I missed that need to preserve this information while building variants of the type. And keeping it in tree.c avoids duplicating this code. Maybe it would make sense to keep it in the middle-end, making the extra field an opaque "extra information" that the middle-end needs to preserve if it ever builds type variants? Although if it clones the function in some way, preserving the linkage may not make that much sense. (I don't have any experience so I may talk nonsense ;-)

This information is supposed to be related to the calling convention (although the g++ ABI uses the same in all cases), which I guess is relevant to pass down to middle and back-end (then it isn't opaque anymore).

The functions like __builtin_memcpy declared in tree.c should probably have "C" linkage. Which means it might be better if I switched to using a non-zero value for "C++" linkage (or at least make build_function_type build a C linkage type by default) so I don't interfere as much with the rest.

So many choices... Well, that's for the next holidays unless someone else wants to work on it.
Comment 39 Marc Glisse 2012-01-04 11:10:30 UTC
Created attachment 26237 [details]
remember linkage of a function type (5)

Updated patch, now bootstraps with configure --enable-languages=c,c++ --disable-werror (except for some __int128 error in libitm which I assume is unrelated). The main difference is that I allow (with a warning) conversions between function pointers that differ by extern "C", so things like calling pthread_create with a C++ function compile. The library changes are still there but most of them are unnecessary if we don't mind warnings.

I didn't try to do the cleanup suggested by Andrew, I was concentrating on the functionality. And I believe the only big functionality missing is giving builtins their right type (__builtin_memcpy is extern "C", operator new[] is "C++", etc).

Then (before even thinking of cleaning up the patch and fixing the whole gcc code base) will come the decision of whether we actually want to do this change. It is necessary in order to conform to the standard, but it is more convenient to keep ignoring linkage...
Comment 40 Jonathan Wakely 2012-01-04 11:49:35 UTC
Great! If all existing code is accepted with a warning that provides backwards compatibility, but also allows conforming code to correctly overload on language linkage - that sounds ideal.

IMHO the warning should be controllable by something such as -Wlanguage-linkage (since there will be lots of warnings in some codebases, so it needs to be possible to use -Wall but disable these warnings) and the conversion should be rejected with -pedantic-errors.
Comment 41 Wolfgang Bangerth 2012-01-04 12:28:13 UTC
I would expect a lot of code to trigger this warning. It is quite common
to pass the address of a static member function to pthread_create but since
there is no way to make a static member function 'extern "C"', I can't see
how to do that without major contortions. I'm rather sure that this will
turn out to be an unpopular warning :-)

W.
Comment 42 Jakub Jelinek 2012-01-04 12:45:15 UTC
Well, perhaps something like:
#ifdef __cplusplus
extern "C++" int __REDIRECT_NTH (pthread_create, (pthread_t *__restrict __newthread, const pthread_attr_t *__restrict __attr,
void *(*__start_routine) (void *),
void *__restrict __arg) __nonnull ((1, 3)), pthread_create);
#endif
(for glibc) could do the trick (and similarly for qsort and other C functions that take callbacks?), still I agree this would be terribly annoying for everybody.
At least this shouldn't be considered for GCC 4.7 at this point.
Comment 43 Marc Glisse 2012-01-04 12:51:55 UTC
(In reply to comment #40)
> Great! If all existing code is accepted with a warning that provides backwards
> compatibility, but also allows conforming code to correctly overload on
> language linkage - that sounds ideal.

Well, not all existing code, just the most common ones. For instance:
template<class>struct is_fun1{static const bool value=false;};
template<class F,class T>struct is_fun1<F(T)>{static const bool value=true;};
will answer false for an extern "C" function type, and I am not sure how I could make it return true without breaking valid code too much (and you can call template<class F,class T>void f(F(T)); with an extern "C" function, but it has lower priority than template<class T> void f(T); or even void f(...);).

But most uses in common code seem to be fine (it is even more lenient than sunCC, which fails to compile gcc because of extern "C"). Note that accepting these broken codes does imply we do the wrong thing on some valid code but I am hoping not too much. As a guiding principle, I tried to allow conversions only when there would be an error otherwise.

> IMHO the warning should be controllable by something such as -Wlanguage-linkage
> (since there will be lots of warnings in some codebases, so it needs to be
> possible to use -Wall but disable these warnings) and the conversion should be
> rejected with -pedantic-errors.

Agreed, there's a lot of cleanup to do...


(In reply to comment #41)
> I would expect a lot of code to trigger this warning. It is quite common
> to pass the address of a static member function to pthread_create but since
> there is no way to make a static member function 'extern "C"', I can't see
> how to do that without major contortions. I'm rather sure that this will
> turn out to be an unpopular warning :-)

libstdc++ does something like that in one place. As Jonathan told earlier, it would make sense to overload pthread_create so it can take a C++ function. The easiest way not to have the warning is to use a cast (not that this is very standard...).

As for the popularity... I am still not sure we actually want to do this at all, this is a proof of concept to help make a decision.
Comment 44 Jonathan Wakely 2012-01-04 13:00:27 UTC
(In reply to comment #42)
> still I agree this would be terribly annoying for
> everybody.

Not everybody, only those who don't also use another compiler that already diagnoses it  ;)

I'd be happy if a warning was only enabled by -pedantic not -Wall, but I accept I'm in a small minority who even know about this problem, and an even smaller minority who want it fixed
Comment 45 Marc Glisse 2012-01-04 13:06:10 UTC
(In reply to comment #42)
> Well, perhaps something like:
> #ifdef __cplusplus
> extern "C++" int __REDIRECT_NTH (pthread_create, (pthread_t *__restrict
> __newthread, const pthread_attr_t *__restrict __attr,
> void *(*__start_routine) (void *),
> void *__restrict __arg) __nonnull ((1, 3)), pthread_create);
> #endif
> (for glibc) could do the trick (and similarly for qsort and other C functions
> that take callbacks?)

For bsearch and qsort, the standard actually requires those (and solaris has them, although we fixinclude them out currently). For posix functions, well, the posix-c++-wg mailing-list hasn't seen a single email since March 2010...

> still I agree this would be terribly annoying for everybody.

Yes.

> At least this shouldn't be considered for GCC 4.7 at this point.

Oh, of course. Maybe not even 4.8. In the patch, I (partially) enabled alias templates in system headers because there are some things in libstdc++ that may not be possible to do without those, so it may be better to wait until -std=c++11 becomes the default if we want to do that. Plus, that completely breaks the ABI, so it should be synchronized with other ABI-breaking changes.

And we may even decide on an official WONTFIX.
Comment 46 xiaoyuanbo 2012-02-22 12:42:46 UTC
i sure you problem primary
Comment 47 Andrew Pinski 2013-03-21 19:42:03 UTC
*** Bug 56683 has been marked as a duplicate of this bug. ***
Comment 48 Harald van Dijk 2014-03-01 18:17:12 UTC
(In reply to Marc Glisse from comment #39)
> Created attachment 26237 [details]
> remember linkage of a function type (5)

I've been experimenting with this (although updated to more recent GCC), and one issue I see, functionality-wise, is what happens when an extern "C" function declaration is followed by a function definition without extern "C" being specified:

extern "C" {
typedef void (*fpt)();
void f();
}
fpt x = f;
void f() { }
fpt y = f;

The initialisation of x is correctly silently accepted, but the initialisation of y warns about an invalid conversion.

As I understand the standard, if the first declaration specifies language linkage, that language linkage is remembered, even if later declarations omit it. The name f does keep C linkage (as seen by nm), but its type loses it.

Note: it is possible that I made a mistake when updating to newer GCC.
Comment 49 Marc Glisse 2014-03-01 18:52:27 UTC
(In reply to Harald van Dijk from comment #48)
> I've been experimenting with this (although updated to more recent GCC), and
> one issue I see, functionality-wise, is what happens when an extern "C"
> function declaration is followed by a function definition without extern "C"
> being specified:

The examples in [dcl.link] seem to agree with your interpretation. It is likely that I missed several cases like this one. As you can see, I haven't worked on this in more than 2 years, and I don't think I'll work on it again any time soon, so feel free to take over (and don't hesitate to consider large pieces of my patch as nonsense). Fixing this particular issue should not be too hard, there must be a place in the compiler that merges a number of properties from the early declaration into the definition, and we need to add extern "C" to that list.
Comment 50 Jorn Wolfgang Rennecke 2014-03-02 15:40:54 UTC
(In reply to Marc Glisse from comment #49)
> large pieces of my patch as nonsense). Fixing this particular issue should
> not be too hard, there must be a place in the compiler that merges a number
> of properties from the early declaration into the definition, and we need to
> add extern "C" to that list.

It's not exactly a single place. For C, in c/c-decl.c, we got
duplicate_decls, which uses merge_decls.

For C++, in cp/decl.c, we got another function called duplicate_decls.
Comment 51 Harald van Dijk 2014-03-08 01:36:23 UTC
(In reply to Marc Glisse from comment #49)
> Fixing this particular issue should
> not be too hard, there must be a place in the compiler that merges a number
> of properties from the early declaration into the definition, and we need to
> add extern "C" to that list.

That does indeed seem simple enough. duplicate_decls can change the type pretty much the same way it can change the name's linkage (except it cannot simply modify the old type; it does have to build a new one, but it can set the new type in the existing declaration), and can do so at the same location. It seems like the right place to do it, since the inheritance of the type's linkage works pretty much the same way as the inheritance of the name's linkage.

Note that a consequence of this is that a function declaration that uses a typedef may not be compatible with the typedef (I think):

extern "C" { void f(); }
typedef void t();
t f, *g = f; // valid redeclaration of f, invalid initialisation of g
extern "C" t f; // invalid redeclaration of f
extern "C++" t f; // invalid redeclaration of f

Linkage conflicts with built-in declarations of functions are also a bit of a problem: implementing this as described makes GCC fail to compile, because of conflicts with built-in functions. The implicit declarations of built-in functions have types with C++ linkage. A simple example:

extern "C" { int (&myputs)(const char *) = __builtin_puts; }

test.cc:1:44: error: invalid initialization of reference of type ‘int (&)(const char*) extern "C"’ from expression of type ‘int(const char*)’
 extern "C" { int (&myputs)(const char *) = __builtin_puts; }
                                            ^

So the same logic to change the a redeclaration's type would need to be applied to built-in function declarations as well.

(I may well continue working on this, but if I do, I will only do so occasionally and will likely not be able to send anything meaningful or useful for a long time.)
Comment 52 Marc Glisse 2014-03-08 08:16:17 UTC
(In reply to Harald van Dijk from comment #51)
> Note that a consequence of this is that a function declaration that uses a
> typedef may not be compatible with the typedef (I think):
> 
> extern "C" { void f(); }
> typedef void t();
> t f, *g = f; // valid redeclaration of f, invalid initialisation of g

Fun!

> extern "C" t f; // invalid redeclaration of f

I am not 100% sure about that one.

> Linkage conflicts with built-in declarations of functions are also a bit of
> a problem:

Yes, as I said in comment #38 and comment #39, giving builtin functions the right linkage is a big missing part of the patch.

> implementing this as described makes GCC fail to compile,

At least in the version of the patch that is attached to the PR, gcc tries to accept almost anything except in some pedantic mode. The goal is to avoid breaking every code on earth, or the patch has no chance of ever being accepted.

> (I may well continue working on this, but if I do, I will only do so
> occasionally and will likely not be able to send anything meaningful or
> useful for a long time.)

Good luck!
Comment 53 Harald van Dijk 2014-04-21 19:38:54 UTC
(In reply to Marc Glisse from comment #52)
> (In reply to Harald van Dijk from comment #51)
> > extern "C" { void f(); }
> > typedef void t();
> > t f, *g = f; // valid redeclaration of f, invalid initialisation of g
> 
> Fun!

What's fun is that in getting this working, I found not only had I accidentally introduced a bug (already fixed -- the typedef got changed, instead of only the function's type), but that exact same bug also affects in Sun C++.

> > extern "C" t f; // invalid redeclaration of f
> 
> I am not 100% sure about that one.

It redeclares f as void(), where the previous declaration gave it type void() extern "C", and the special exception in [dcl.link] to inherit a previous declaration's linkage (both of the name and of the type) doesn't apply when the linkage is explicitly specified, right?

> > Linkage conflicts with built-in declarations of functions are also a bit of
> > a problem:
> 
> Yes, as I said in comment #38 and comment #39, giving builtin functions the
> right linkage is a big missing part of the patch.

I've got this part working, and found it much easier to reverse the logic: all code using build_function_type gets a C function type. It required just a few more changes of build_function_type to build_function_type2 in the C++ frontend.

> > implementing this as described makes GCC fail to compile,
> 
> At least in the version of the patch that is attached to the PR, gcc tries
> to accept almost anything except in some pedantic mode. The goal is to avoid
> breaking every code on earth, or the patch has no chance of ever being
> accepted.

I've given it a try on a large number of packages (including Firefox, Chromium, Qt+KDE), and I'm down to five categories of hard errors, four of which IMO should be warnings, but comments on that would be welcome.

1:

extern "C" typedef void (*fpt1)() __attribute__((noreturn));
void f1();
fpt1 fp1 = f1; // error: invalid conversion from ‘void (*)()’ to ‘fpt1 {aka void (*)()__attribute__((noreturn)) extern "C"}’ [-fpermissive]

This should work, with a warning. The logic to allow an implicit conversion to the corresponding function type with different linkage doesn't work when the attributes differ.

2:

extern "C" typedef void (*fpt2)();
template <typename> void f2();
fpt2 fp2 = f2<int>; // error: no matches converting function ‘f2’ to type ‘fpt2 {aka void (*)() extern "C"}’

This should work, with a warning. This possibly has the same underlying cause as what I reported as PR60531, namely that f2<int> is considered an overloaded function even though it isn't.

3:

extern "C" void f3();
template <void()> struct S3 { };
template struct S3<f3>;

This should work, with a warning. I was worried this might cause a conflict between a C function template argument and a C++ function template argument with the same name, but that cannot ever happen, the only possible way that they could get the same mangled name is if the code is already invalid for other reasons.

4:

extern "C" void f4();
struct S4 {
  S4(void());
private:
  S4(const S4 &);
};
S4 s4(f4);

This should work, with a warning, but only after all standard means of constructing S4 have failed. In particular, if another constructor is added that can accept f4's type (say S4(...), or S4(bool)), the other constructor needs to be picked.

5:

extern "C" void f5(void());
void f5(void());
void g5() { long p = (long)f5; }

This should be considered acceptable breakage. The function f5 in question is actually atexit, which in current glibc is not an overloaded function, but would become an overloaded function, as specified in the standard. Taking its address without any context doesn't give enough information to determine which overload's address should be taken (even though they both have the same address).

P.S.: the patch here contains a comment questioning the validity of extern "C" template aliases. I think that a _lot_ more is valid than is currently accepted:

7.5p1:
All function types, function names with external linkage, and variable names with external linkage have a language linkage.

14p4:
A template name has linkage (3.5). A non-member function template can have internal linkage; any other template name shall have external linkage. [...] A template, a template explicit specialization (14.7.3), and a class template partial specialization shall not have C linkage.

A static template function has internal linkage, so doesn't have external linkage, so doesn't have language linkage, so never has C linkage, even if it appears in an extern "C" block. Template classes and template aliases aren't function types, so also never have language linkage, so never have C linkage.

The restriction on class template partial specializations suggests that this isn't what the standard intends (they never have C linkage, so why specify that they cannot have C linkage?), but it's useful, and I don't see any other interpretation based on the actual wording, so that is what I've implemented. This avoids the need for some of the uses of template aliases in libstdc++. If desired, the useful templates that were previously rejected could get a pedwarn rather than a hard error.