Alex Merry | 1 Oct 23:56

[PATCH] KSharedPtr operator== problem

KSharedPtr's two non-member operators (operator==(T*, const KSharedPtr<T>&) 
and the equivalent operator!=) are not friends of the class, so these 
comparisons fail to compile.

Since some forward-declarations and friending of template functions is 
involved, I wanted to make sure
  (a) I didn't do anything stupid and
  (b) no-one knows of any problems with old gcc versions or other platforms
before I commit, especially as it's a candidate for 4.1.3.

Anyone see any issues?

Patch attached.

Alex

--

-- 
Proud KDE hacker: http://www.kde.org
Get KDE 4.1 - out now!

Index: ksharedptr.h
===================================================================
--- ksharedptr.h	(revision 866760)
+++ ksharedptr.h	(working copy)
@@ -33,6 +33,10 @@

 typedef QSharedData KShared;

(Continue reading)

Thiago Macieira | 2 Oct 08:27

Re: [PATCH] KSharedPtr operator== problem

Alex Merry wrote:
>KSharedPtr's two non-member operators (operator==(T*, const
> KSharedPtr<T>&) and the equivalent operator!=) are not friends of the
> class, so these comparisons fail to compile.
>
>Since some forward-declarations and friending of template functions is
>involved, I wanted to make sure
>  (a) I didn't do anything stupid and
>  (b) no-one knows of any problems with old gcc versions or other
> platforms before I commit, especially as it's a candidate for 4.1.3.
>
>Anyone see any issues?

Yes.

Template friends are not supported in any compiler but gcc. You'll have to 
expose the privates as public for all other compilers.

--

-- 
  Thiago Macieira  -  thiago (AT) macieira.info - thiago (AT) kde.org
    PGP/GPG: 0x6EF45358; fingerprint:
    E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
Alex Merry | 4 Oct 00:27

Re: [PATCH] KSharedPtr operator== problem

On Thursday 02 October 2008 07:27:25 Thiago Macieira wrote:
> Alex Merry wrote:
> >KSharedPtr's two non-member operators (operator==(T*, const
> > KSharedPtr<T>&) and the equivalent operator!=) are not friends of the
> > class, so these comparisons fail to compile.

> Template friends are not supported in any compiler but gcc. You'll have to
> expose the privates as public for all other compilers.

Good job I checked, then.

How does the attached look?  I'm assuming that no-one would try compiling one 
part of KDE under gcc and another under a different compiler (since I would 
guess that way lies madness).

Alex

--

-- 
Proud KDE hacker: http://www.kde.org
Get KDE 4.1 - out now!

Index: ksharedptr.h
===================================================================
--- ksharedptr.h	(revision 866760)
+++ ksharedptr.h	(working copy)
@@ -33,6 +33,10 @@

 typedef QSharedData KShared;
(Continue reading)

Adriaan de Groot | 4 Oct 01:43

Re: [PATCH] KSharedPtr operator== problem

On Saturday 04 October 2008 00:27:09 Alex Merry wrote:
> How does the attached look?

I was going to write "that way lies madness" as a response before having read 
your whole message, but apparently that was drummed in sub-conciously. 

> I'm assuming that no-one would try compiling
> one part of KDE under gcc and another under a different compiler (since I
> would guess that way lies madness).

It just doesn't seem like a good idea to wriggle around like that, changing 
protected for public. I looks to me like a bit of ugliness that is just asking 
for maintainence trouble.

But in this particular case I'd like to doubt Thiago's words: template friends 
are documented to work in MSVC [1], IBM XL C++ [2]. I may be missing a 
subtlety here, though. I haven't actually tried compiling KSharedPtr with the 
earlier proposed patch in Sun Studio, though. I'll try it in the morning. 
Suffice to say the proposed workaround doesn't seem nice at all to me. Don't 
forget that the range of KDE-supported compilers is smaller than the range of 
Qt supported compilers (right?) so we need to just check those -- and the C++ 
standard. A quick search didn't turn up any chapter and verse saying you can't 
have template friends.

[1] http://www.cs.fiu.edu/~weiss/Deltoid/msvc_stl_3.html , scroll down to 3.6
[2] 
http://publib.boulder.ibm.com/infocenter/lnxpcomp/v8v101/index.jsp?topic=/com.ibm.xlcpp8l.doc/language/ref/friends_and_templates.htm

Thiago Macieira | 4 Oct 09:29

Re: [PATCH] KSharedPtr operator== problem

Adriaan de Groot wrote:
>But in this particular case I'd like to doubt Thiago's words: template
> friends are documented to work in MSVC [1], IBM XL C++ [2]. I may be
> missing a subtlety here, though. I haven't actually tried compiling
> KSharedPtr with the earlier proposed patch in Sun Studio, though. I'll
> try it in the morning. Suffice to say the proposed workaround doesn't
> seem nice at all to me. Don't forget that the range of KDE-supported
> compilers is smaller than the range of Qt supported compilers (right?)
> so we need to just check those -- and the C++ standard. A quick search
> didn't turn up any chapter and verse saying you can't have template
> friends.

The range of KDE-supported compilers is a subset (not just smaller) of the 
range of Qt supported compilers.

I tried this technique for QSharedPointer (Qt 4.5 code) and the result was 
quite clear: only gcc supported it. Not even the latest versions of 
Microsoft Visual Studio, Intel CC or HP-UX's aCC support it. 
Unfortunately, I couldn't test the latest of SunStudio, but SunCC 5.5 
doesn't support either.

I asked our C++ standard experts in-house about the subject. Roberto 
couldn't come up with a definitive answer: we can't decide whether the 
standard allows it or not.

In any case, the solution is very simple anyways, invert the comparison:

template <class T> bool operator== (const T* p, const KSharedPtr<T>& o)
{ return o == p; }
template <class T> bool operator!= (const T* p, const KSharedPtr<T>& o)
(Continue reading)

Adriaan de Groot | 4 Oct 10:08

Re: [PATCH] KSharedPtr operator== problem

On Saturday 04 October 2008 09:29:12 Thiago Macieira wrote:
> I asked our C++ standard experts in-house about the subject. Roberto
> couldn't come up with a definitive answer: we can't decide whether the
> standard allows it or not.

OK. At some point "does it compile with the tools we have?" trumps "does the 
standard allow it?" anyway. There must be some subtlety I'm missing there.

What would help in this specific case is a complete test program (from Alex) 
(with no dependency on an installed KDE, if possible) that exercises this 
stuff so we can just throw it at the non-gcc compilers and see what happens. 

> In any case, the solution is very simple anyways, invert the comparison:

Elegance trumps dealing-with-possibly-supported-template-features every time.

[ade]

Thiago Macieira | 4 Oct 11:55

Re: [PATCH] KSharedPtr operator== problem

Adriaan de Groot wrote:
>On Saturday 04 October 2008 09:29:12 Thiago Macieira wrote:
>> I asked our C++ standard experts in-house about the subject. Roberto
>> couldn't come up with a definitive answer: we can't decide whether the
>> standard allows it or not.
>
>OK. At some point "does it compile with the tools we have?" trumps "does
> the standard allow it?" anyway. There must be some subtlety I'm missing
> there.
>
>What would help in this specific case is a complete test program (from
> Alex) (with no dependency on an installed KDE, if possible) that
> exercises this stuff so we can just throw it at the non-gcc compilers
> and see what happens.

Try this:

template<class T> class MyClass;
template<class T> void friendFunction(MyClass<T> &t);

template<class T> class MyClass
{
    template<class X> friend void friendFunction(MyClass<T> t);
    template<class X> friend class MyClass;
    T i;
public:
    MyClass() : i(1) { }

    template<class X> void set(MyClass<X> x)
    { i = x.i; }
(Continue reading)

Re: [PATCH] KSharedPtr operator== problem

Am Samstag 04 Oktober 2008 schrieb Thiago Macieira:
> Adriaan de Groot wrote:
> >On Saturday 04 October 2008 09:29:12 Thiago Macieira wrote:
> >> I asked our C++ standard experts in-house about the subject. Roberto
> >> couldn't come up with a definitive answer: we can't decide whether the
> >> standard allows it or not.
> >
> >OK. At some point "does it compile with the tools we have?" trumps "does
> > the standard allow it?" anyway. There must be some subtlety I'm missing
> > there.
> >
> >What would help in this specific case is a complete test program (from
> > Alex) (with no dependency on an installed KDE, if possible) that
> > exercises this stuff so we can just throw it at the non-gcc compilers
> > and see what happens.
>
> Try this:
>
> ...
> This works fine in GCC 4.3, but fails in 3.3.6:
> /dev/stdin: In function `void friendFunction(MyClass<T>&) [with T = int]':
> /dev/stdin:19:   instantiated from here
> /dev/stdin:5: error: `int MyClass<int>::i' is private
> /dev/stdin:14: error: within this context
>
> I added the macro Q_NO_TEMPLATE_FRIENDS to qglobal.h to test this compiler
> feature.
>
> BTW, QSharedPointer also requires partial template specialisation and
> member template functions. That excludes a lot of old compilers from using
(Continue reading)

Thiago Macieira | 4 Oct 12:49

Re: [PATCH] KSharedPtr operator== problem

Christoph Bartoschek wrote:
>? ? //The friend is defined within the class. But this is not a
>? ? //member function.
>? ? friend void friendFunction(MyClass & t) {
>? ? ? ++t.i;
>? ? }

That's not always possible. Imagine it were a conversion function from two 
different template types, and the second one isn't defined yet.

And it doesn't cover the case for another template class (not MyClass).

--

-- 
  Thiago Macieira  -  thiago (AT) macieira.info - thiago (AT) kde.org
    PGP/GPG: 0x6EF45358; fingerprint:
    E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
Alex Merry | 9 Oct 00:13

Re: [PATCH] KSharedPtr operator== problem

On Saturday 04 October 2008 08:29:12 Thiago Macieira wrote:
> In any case, the solution is very simple anyways, invert the comparison:
>
> template <class T> bool operator== (const T* p, const KSharedPtr<T>& o)
> { return o == p; }
> template <class T> bool operator!= (const T* p, const KSharedPtr<T>& o)
> { return o != p; }

*slaps forehead*

Committed r869408.

Alex

--

-- 
Proud KDE hacker: http://www.kde.org
Get KDE 4.1 - out now!


Gmane