User account creation filtered due to spam.

Bug 38392 - Template friend function injection
Summary: Template friend function injection
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.2.3
: P3 normal
Target Milestone: 4.6.0
Assignee: Jason Merrill
URL:
Keywords: link-failure
Depends on:
Blocks:
 
Reported: 2008-12-04 02:37 UTC by Fetsel
Modified: 2010-05-04 04:54 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 3.3.3 4.4.0 4.0.2 2.95.3
Last reconfirmed: 2009-12-24 21:47:32


Attachments
patch (749 bytes, patch)
2009-12-24 21:48 UTC, Jason Merrill
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fetsel 2008-12-04 02:37:06 UTC
When defining a friend function in a template class it does not get correctly defined if the class instantiation comes after the function has been already called. Note: this happens even if you pre-declare the function in its correct context before the same function is called. 
The following is a code sample that reproduces the bug. Note that this has been already discussed and concluded that this is valid c++ standard-compliant code (precisely here: http://groups.google.com/group/comp.lang.c++/browse_frm/thread/493afd501c807ffe#).

-- code --

void Function();

int main(int argc, char* argv[])
{
        Function(); // This does not work

}

template <typename T>
class Test
{
public:
        friend void Function() { printf("Function()"); getchar(); }

};

template class Test<int>;

-- end code --
Comment 1 Fetsel 2008-12-16 01:03:03 UTC
Any news?
Comment 2 Andrew Pinski 2008-12-16 01:08:10 UTC
Confirmed, not a regression.  If you swap around main and the template, the link works correctly.
Comment 3 Fetsel 2009-01-31 20:28:33 UTC
Sorry for the late reply!
I thought I'd receive an e-mail when one of my reports gets updated, silly me.

Yes I know it works if you switch the order, but that's exactly the point of the code, to have them in that specific order and there's no reason why the code as it is shouldn't work being it standard c++ right?
Comment 4 Wolfgang Bangerth 2009-02-01 05:07:58 UTC
Confirmed indeed, with this (linker) error message:

g/x> c++ x.cc
/tmp/ccjPvb3J.o: In function `main':
x.cc:(.text+0x12): undefined reference to `Function()'
collect2: ld returned 1 exit status

This already fails with gcc2.95.

W.
Comment 5 Fetsel 2009-09-15 07:37:44 UTC
Any update about this?
Comment 6 Fetsel 2009-12-22 21:52:14 UTC
Did anyone try this against 4.5? Considering that at this stage only bugfixes are accepted in the codebase for the next release I'd really like to see a possible fix to this in. I tried to compile GCC 4.5 on my machine without much success.
Comment 7 Paolo Carlini 2009-12-22 22:15:20 UTC
Doesn't link with 4.5.0. And doesn't link with ICC and SunStudio either, thus, I'm rather skeptical it should. I also skimmed quickly through the discussion on comp.lang.c++ and didn't notice any neat statement... maybe should look better.
Comment 8 Jonathan Wakely 2009-12-23 11:02:07 UTC
See http://www.open-std.org/jtc1/sc22/wg21/prot/14882fdis/cwg_defects.html#329

I believe a "use" of the function must come after the definition for it to be instantiated, indeed if I add this after the explicit instantiation it links OK:

void(*pf)() = &Function;

Comment 9 Fetsel 2009-12-24 14:48:36 UTC
I know if you move the function it links (btw your link asks me for an HTTP login), but if you follow the discussion in the newsgroup it was concluded that this (the above) is actually perfectly valid standard C++ code, and anyway I think at least the error the linker gives could be more descriptive. Also the fact that other compilers fail with this code it's just because it's fairly rarely used code. For instance, Open Watcom compiled and linked the code perfectly fine without issues (and the executable was working).
Comment 10 Paolo Carlini 2009-12-24 15:39:25 UTC
Try this one:

  http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#329

Anyway, if you could point us to the specific sentence in the thread saying that it's legal, it would be useful. And, well, frankly I would not trust Watcom more than Intel and Sun (and GCC) together ;) But let's CC Jason, maybe he wants to give his opinion...
Comment 11 Fetsel 2009-12-24 17:23:58 UTC
This is possibly the part in which gets confirmed that the code is standard compliant, although it reports the exact same paragraph you linked (the old version): http://groups.google.com/group/comp.lang.c++/tree/browse_frm/thread/493afd501c807ffe/68c709135884bac7?rnum=11&_done=%2Fgroup%2Fcomp.lang.c%2B%2B%2Fbrowse_frm%2Fthread%2F493afd501c807ffe%3F#doc_38f8441247122dcd

I guess that page is some kind of errata corrige? 
Anyway maybe I'm missing something but I see that as a conflict. If I recall correctly it is stated that the context of a injected friend function must not be ambiguous, thus meaning you have to declare it first in the correct namespace. The idea of declaration is that you inform the compiler that the function exists somewhere so that you can use it without having it defined yet. If friend function injection is just like defining the function in the parent namespace of the class and if a template exists as a type (at linking level) only when you create an instance of that type, it means the moment you instantiate the template, then the type is defined and consequently the friend function is injected, thus matching with the declaration done before, thus matching with the call that used that declaration.
As I pointed in that newsgroup thread, it works fine with normal classes, and theoretically a template is just a class in the second you create an instance of it. I don't precisely understand why there should be an exception for templates. I understand not defining the function if you don't create any instance of the template, but otherwise it's a bit unclear to me why there should be this exception. 
Comment 12 Jason Merrill 2009-12-24 21:48:52 UTC
Created attachment 19387 [details]
patch

Here's a fix.  I'm going to hold off on applying it for now since it isn't a regression.
Comment 13 Paolo Carlini 2009-12-24 21:55:36 UTC
Cool. Should the testcase use dg-do link?
Comment 14 Jason Merrill 2009-12-24 22:14:20 UTC
Ah, good point.  I've updated the patch accordingly in my local pre-4.6 git branch.
Comment 15 Fetsel 2009-12-24 22:15:39 UTC
Hey thank you! I'd like to test the patch if I only I'd be able to compile 4.5 successfully. You have any idea on when could this patch make it to a final release?
Comment 16 Paolo Carlini 2009-12-25 09:53:11 UTC
As Jason confirmed, this is not a regression, thus, post 4.5.0.
Comment 17 Fetsel 2009-12-25 10:42:28 UTC
Ok I managed to compile GCC 4.5, applied the patch and compiled the test code above and everything works fine. Thanks again!
And yes, I imagined it would have been post 4.5 but I meant 'when' from a time frame point of view, that's why I said "you have any idea" as I know it's hard to predict a specific time for release.
Comment 18 Paolo Carlini 2009-12-25 10:48:26 UTC
If I were you, I would not use this kind of C++ at all, for the time being. As we discussed already, it's *very* weakly supported and your software would not be portable.
Comment 19 Fetsel 2009-12-25 11:02:36 UTC
Yeah I know, but consider that I used this code only in a specific circumstance and does not constitute a core part of my application (or a foundation of some higher level mechanism). And I know it's a lot like an exploit but it does allow me to add a tiny yet very slick feature to my code. 
Also I checked and they fixed this in visual c++ 2010 as well (after my report), so having the two major compilers supporting this code is good enough for me.
Comment 20 Paolo Carlini 2009-12-25 11:06:06 UTC
Then wait, good luck
Comment 21 Jason Merrill 2010-04-07 15:55:18 UTC
Subject: Bug 38392

Author: jason
Date: Wed Apr  7 15:54:42 2010
New Revision: 158073

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158073
Log:
	PR c++/38392
	* pt.c (tsubst_friend_function): Instatiate a friend that has already
	been used.

Added:
    trunk/gcc/testsuite/g++.dg/template/friend51.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/pt.c
    trunk/gcc/testsuite/ChangeLog

Comment 22 Jason Merrill 2010-05-04 04:54:10 UTC
Fixed for 4.6.