221316 - nsCOMPtr operator== doesn't handle multiple inheritance correctly

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Brian Ryner (not reading)]

Here's an example:

class Foo : public nsIFoo, public nsIBar { ... };

void Foo::someFunction()
{
  nsCOMPtr<nsIBar> ptr(this);
  if (ptr == this) { ... }
}

The comparison will return false because nsCOMPtr casts both arguments to void*.

Comment 1

[Brian Ryner (not reading)]

Attachment: fix suggested by dbaron

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 132696 [details] [diff] [review]
fix suggested by dbaron

Looks fine to me, except that for the last for I'd rather skip the
NS_CONST_CASTs and leave them un-cast.

Comment 3

[Darin Fisher]

Comment on attachment 132696 [details] [diff] [review]
fix suggested by dbaron

sr=darin with the same comment as dbaron.  i think the code would be clearer
without the unnecessary NS_CONST_CAST expressions.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, I take that back.  Leave the NS_CONST_CASTs.  They might help some
compilers when converting arguments.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 132696 [details] [diff] [review]
fix suggested by dbaron

>@@ -1216,7 +1216,7 @@ inline
> NSCAP_BOOL
> operator==( U* lhs, const nsCOMPtr<T>& rhs )
>   {
>-    return NS_STATIC_CAST(void*, lhs) == NS_STATIC_CAST(const void*, rhs.get());
>+    return NS_CONST_CAST(U*, lhs) == NS_STATIC_CAST(const T*, rhs.get());
>   }
> 

And fix this one so it casts to |const U*|. :-)

Comment 6

[Nick Blievers]

This change breaks MipsPro (7.4.1) on IRIX:

cc-1042 CC: ERROR File = ../../../../dist/include/xpcom/nsCOMPtr.h, Line = 1211
  The types of operands "const nsIHTMLCSSStyleSheet *" and
          "const nsDerivedSafe<nsIStyleSheet> *" are incompatible.

      return NS_STATIC_CAST(const T*, lhs.get()) == NS_CONST_CAST(const U*, rhs);
                                                 ^
          detected during instantiation of "NSCAP_BOOL operator==(const
                    nsCOMPtr<nsIHTMLCSSStyleSheet> &,
                    nsDerivedSafe<nsIStyleSheet> *)" at line 782 of
                    "/build2/tinderbox/mozilla/build/IRIX_6.5_Depend/mozilla/co
                    ntent/xml/document/src/nsXMLDocument.cpp"

cc-1042 CC: ERROR File = ../../../../dist/include/xpcom/nsCOMPtr.h, Line = 1211
  The types of operands "const nsIHTMLStyleSheet *" and
          "const nsDerivedSafe<nsIStyleSheet> *" are incompatible.

      return NS_STATIC_CAST(const T*, lhs.get()) == NS_CONST_CAST(const U*, rhs);
                                                 ^
          detected during instantiation of "NSCAP_BOOL operator==(const
                    nsCOMPtr<nsIHTMLStyleSheet> &,
                    nsDerivedSafe<nsIStyleSheet> *)" at line 835 of
                    "/build2/tinderbox/mozilla/build/IRIX_6.5_Depend/mozilla/co
                    ntent/xml/document/src/nsXMLDocument.cpp"

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That's been fixed.  Look at tinderbox.  And see also bug 221525.

Comment 8

[Nick Blievers]

Great, thanks :-)

Comment 9

[Brian Ryner (not reading)]

checked in, and I think we got the bustage under control.