801572 - nsDataObj.cpp not compiling on VC11

Status: RESOLVED DUPLICATE of bug 801471

(Core :: Widget: Win32, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Fix compile errors on Win8 VC11

The nsDataObj.cpp compilation started failing on Windows 8 for me because of implicit conversion between HRESULT and nsresult because of NS_ENSURE_TRUE. I'm unsure why this is started happening now as I can't find changes that obviously would have caused it. Attached is a patch that 'fixes' the problem.

Comment 1

[Bas Schouten (:bas.schouten)]

Also related to 801579. There's more failures in nsHTMLEditor.cpp as well:

nsHTMLEditor.cpp
c:\Users\Bas\Dev\mozilla-inbound\config\rules.mk:984:0$ c:/Users/Bas/Dev/mi-debu
g/_virtualenv/Scripts/python.exe -O ../../../../mozilla-inbound/build/cl.py cl -
FonsHTMLEditor.obj -c -D_HAS_EXCEPTIONS=0 -I../../../dist/stl_wrappers  -D_IMPL_
NS_LAYOUT -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_A
PI -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES
-I../../../../mozilla-inbound/editor/libeditor/html -I. -I../../../dist/include
 -Ic:/Users/Bas/Dev/mi-debug/dist/include/nspr -Ic:/Users/Bas/Dev/mi-debug/dist/
include/nss     -I../../../../mozilla-inbound/editor/libeditor/base -I../../../.
./mozilla-inbound/editor/libeditor/text -I../../../../mozilla-inbound/editor/txm
gr/src -I../../../../mozilla-inbound/content/base/src -I../../../../mozilla-inbo
und/layout/style     -TP -nologo -W3 -Gy -Fdgenerated.pdb -wd4345 -wd4351 -wd480
0 -we4553 -GR-  -DDEBUG -D_DEBUG -DTRACING -Zi -Oy-  -MDd           -FI ../../..
/dist/include/mozilla-config.h -DMOZILLA_CLIENT  c:/Users/Bas/Dev/mi-debug/edito
r/libeditor/html/../../../../mozilla-inbound/editor/libeditor/html/nsHTMLEditor.
cpp
nsHTMLEditor.cpp
c:\users\bas\dev\mi-debug\dist\include\FrameMetrics.h(89) : warning C4244: 'argu
ment' : conversion from 'gfxFloat' to 'mozilla::gfx::Float', possible loss of da
ta
c:/Users/Bas/Dev/mi-debug/editor/libeditor/html/../../../../mozilla-inbound/edit
or/libeditor/html/nsHTMLEditor.cpp(769) : error C2678: binary '==' : no operator
 found which takes a left-hand operand of type 'mozilla::DebugOnly<T>' (or there
 is no acceptable conversion)
        with
        [
            T=nsresult
        ]
        c:\users\bas\dev\mi-debug\dist\include\nsTSubstring.h(851): could be 'bo
ol operator ==(const nsAString_internal::base_string_type &,const nsAString_inte
rnal::base_string_type &)'
        c:\users\bas\dev\mi-debug\dist\include\nsTSubstring.h(851): or       'bo
ol operator ==(const nsACString_internal::base_string_type &,const nsACString_in
ternal::base_string_type &)'
        c:\users\bas\dev\mi-debug\dist\include\nsChangeHint.h(126): or       'vo
id operator ==(nsChangeHint,nsChangeHint)'
        c:\users\bas\dev\mi-debug\dist\include\gfxFontFeatures.h(24): or       '
bool operator ==(const gfxFontFeature &,const gfxFontFeature &)'
        c:\users\bas\dev\mozilla-inbound\editor\libeditor\html\nsWSRunObject.h(6
9): or       'bool operator ==(const WSType &,const WSType &)'
        while trying to match the argument list '(mozilla::DebugOnly<T>, tag_nsr
esult)'
        with
        [
            T=nsresult
        ]
c:\Users\Bas\Dev\mozilla-inbound\config\rules.mk:984:0: command 'c:/Users/Bas/De
v/mi-debug/_virtualenv/Scripts/python.exe -O ../../../../mozilla-inbound/build/c
l.py cl -FonsHTMLEditor.obj -c -D_HAS_EXCEPTIONS=0 -I../../../dist/stl_wrappers
 -D_IMPL_NS_LAYOUT -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPO
RT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_
THEBES   -I../../../../mozilla-inbound/editor/libeditor/html -I. -I../../../dist
/include  -Ic:/Users/Bas/Dev/mi-debug/dist/include/nspr -Ic:/Users/Bas/Dev/mi-de
bug/dist/include/nss     -I../../../../mozilla-inbound/editor/libeditor/base -I.
./../../../mozilla-inbound/editor/libeditor/text -I../../../../mozilla-inbound/e
ditor/txmgr/src -I../../../../mozilla-inbound/content/base/src -I../../../../moz
illa-inbound/layout/style     -TP -nologo -W3 -Gy -Fdgenerated.pdb -wd4345 -wd43
51 -wd4800 -we4553 -GR-  -DDEBUG -D_DEBUG -DTRACING -Zi -Oy-  -MDd           -FI
 ../../../dist/include/mozilla-config.h -DMOZILLA_CLIENT  c:/Users/Bas/Dev/mi-de
bug/editor/libeditor/html/../../../../mozilla-inbound/editor/libeditor/html/nsHT
MLEditor.cpp' failed, return code 2

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

(In reply to Bas Schouten (:bas) from comment #1)
> c:/Users/Bas/Dev/mi-debug/editor/libeditor/html/../../../../mozilla-inbound/
> editor/libeditor/html/nsHTMLEditor.cpp(769) : error C2678: binary '==' : no
> operator found which takes a left-hand operand of type 'mozilla::DebugOnly<T>' (or
> there is no acceptable conversion)
>         with
>         [
>             T=nsresult
>         ]
>         c:\users\bas\dev\mi-debug\dist\include\nsTSubstring.h(851): could be
> 'bool operator ==(const nsAString_internal::base_string_type &,const nsAString_internal::base_string_type &)'
>         c:\users\bas\dev\mi-debug\dist\include\nsTSubstring.h(851): or      
> 'bool operator ==(const nsACString_internal::base_string_type &,const
> nsACString_internal::base_string_type &)'
>         c:\users\bas\dev\mi-debug\dist\include\nsChangeHint.h(126): or      
> 'void operator ==(nsChangeHint,nsChangeHint)'
>         c:\users\bas\dev\mi-debug\dist\include\gfxFontFeatures.h(24): or    
> 'bool operator ==(const gfxFontFeature &,const gfxFontFeature &)'
>        
> c:\users\bas\dev\mozilla-inbound\editor\libeditor\html\nsWSRunObject.h(69): or
> 'bool operator ==(const WSType &,const WSType &)'
>         while trying to match the argument list '(mozilla::DebugOnly<T>,
> tag_nsresult)'
>         with
>         [
>             T=nsresult
>         ]

Is this a bug in MSVC?  DebugOnly<T> has an implicit conversion operator to T, so the built-in equality comparison between nsresult and itself should work fine here.  If not, I suppose the right fix is to define an explicit operator== for DebugOnly<T>, but we shouldn't have to do that.  (This specific case is most easily fixed by using MOZ_ALWAYS_TRUE instead of MOZ_ASSERT, which will remove the need for the DebugOnly, but that will leave the general problem.)

Comment 3

[Bas Schouten (:bas.schouten)]

(In reply to :Aryeh Gregor from comment #2)
> (In reply to Bas Schouten (:bas) from comment #1)
> > c:/Users/Bas/Dev/mi-debug/editor/libeditor/html/../../../../mozilla-inbound/
> > editor/libeditor/html/nsHTMLEditor.cpp(769) : error C2678: binary '==' : no
> > operator found which takes a left-hand operand of type 'mozilla::DebugOnly<T>' (or
> > there is no acceptable conversion)
> >         with
> >         [
> >             T=nsresult
> >         ]
> >         c:\users\bas\dev\mi-debug\dist\include\nsTSubstring.h(851): could be
> > 'bool operator ==(const nsAString_internal::base_string_type &,const nsAString_internal::base_string_type &)'
> >         c:\users\bas\dev\mi-debug\dist\include\nsTSubstring.h(851): or      
> > 'bool operator ==(const nsACString_internal::base_string_type &,const
> > nsACString_internal::base_string_type &)'
> >         c:\users\bas\dev\mi-debug\dist\include\nsChangeHint.h(126): or      
> > 'void operator ==(nsChangeHint,nsChangeHint)'
> >         c:\users\bas\dev\mi-debug\dist\include\gfxFontFeatures.h(24): or    
> > 'bool operator ==(const gfxFontFeature &,const gfxFontFeature &)'
> >        
> > c:\users\bas\dev\mozilla-inbound\editor\libeditor\html\nsWSRunObject.h(69): or
> > 'bool operator ==(const WSType &,const WSType &)'
> >         while trying to match the argument list '(mozilla::DebugOnly<T>,
> > tag_nsresult)'
> >         with
> >         [
> >             T=nsresult
> >         ]
> 
> Is this a bug in MSVC?  DebugOnly<T> has an implicit conversion operator to
> T, so the built-in equality comparison between nsresult and itself should
> work fine here.  If not, I suppose the right fix is to define an explicit
> operator== for DebugOnly<T>, but we shouldn't have to do that.  (This
> specific case is most easily fixed by using MOZ_ALWAYS_TRUE instead of
> MOZ_ASSERT, which will remove the need for the DebugOnly, but that will
> leave the general problem.)

I think there's something about how many conversion levels can be implicit or something, right? I can never remember.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I think a maximum of one implicit conversion per argument will be performed, yes.  But there's only one implicit conversion here.  Notice how on all other platforms it compiles just fine.

Comment 5

[Bas Schouten (:bas.schouten)]

(In reply to :Aryeh Gregor from comment #4)
> I think a maximum of one implicit conversion per argument will be performed,
> yes.  But there's only one implicit conversion here.  Notice how on all
> other platforms it compiles just fine.

In my experience that's an extremely poor argument for certain functionality being in the C++ spec :-).

Comment 6

[Robert Longson [:longsonr]]

This looks very similar to bug 801471

Comment 7

[(no longer active)]

Comment 0 is not a bug in VC11, it's just that VC11 implements enum classes, and is therefore catching this bug which is real (HRESULT != nsresult.)

Comment 9

[(no longer active)]

(In reply to Bas Schouten (:bas) from comment #1)
> Also related to 801579. There's more failures in nsHTMLEditor.cpp as well:

This one seems like a compiler bug?  CCing Rafael to see if he has any ideas.

Comment 10

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

(In reply to Ehsan Akhgari [:ehsan] from comment #7)
> Comment 0 is not a bug in VC11, it's just that VC11 implements enum classes,
> and is therefore catching this bug which is real (HRESULT != nsresult.)

Yeah, I was talking about comment 1 -- that looks like a compiler bug.

Comment 11

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

I don't think I have a lot to add. Does this reproduce with windows 7 at all? If not, can someone try to create a reduced testcase?

With a reduced testcase I can try to hunt down in the spec which compiler is wrong, but given that we cannot fix vc11, we probably have to implement the overloaded == Aryeh suggested.