212919 - MSVC++ .net 2003: Breaks with nsAutoPtr<T>

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Stephen Walker]

A current trunk build breaks when compiling nsCategoryManager.cpp:

cl -FonsCategoryManager.obj -c -DOSTYPE=\"WINNT5.1\" -DOSARCH=\"WINNT\"
-D_IMPL_NS_COM -DEXPORT_XPTI_API -Ic:/Mozilla/mozilla/xpco
m/components/../base -Ic:/Mozilla/mozilla/xpcom/components/../thread
-Ic:/Mozilla/mozilla/xpcom/components/../ds -I../../dist/incl
ude/string -I../../dist/include/xpcom -I../../dist/include
-I../../dist/include/nspr          -TP -nologo -W3 -nologo -Gy -Fdxpcom
components_s.pdb  -DNDEBUG -DTRIMMED -O2 -MD            -DMOZILLA_CLIENT
-D_MOZILLA_CONFIG_H_ -DX_DISPLAY_MISSING=1 -DHAVE_SNPRINT
F=1 -D_WINDOWS=1 -D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1
-DWINVER=0x400 -DSTDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN
=1 -DNO_X11=1 -D_X86_=1 -DD_INO=d_ino -DMOZ_DEFAULT_TOOLKIT=\"windows\"
-DMOZ_STATIC_MAIL_BUILD=1 -DMOZ_THUNDERBIRD=1 -DMOZ_STATIC
_MAIL_BUILD=1 -DMOZ_XUL_APP=1 -DMOZ_ENABLE_COREXFONTS=1 -DIBMBIDI=1
-DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DMOZ_LOGGING=1 -DMOZ_USER_
DIR=\"Mozilla\" -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_DLL_SUFFIX=\".dll\"
-DJS_THREADSAFE=1 -DNS_PRINT_PREVIEW=1 -DNS_PRINTING=
1 -DMOZILLA_VERSION=\"1.5b\" 
c:/Mozilla/mozilla/xpcom/components/nsCategoryManager.cpp
nsCategoryManager.cpp
c:\Mozilla\mozilla\xpcom\ds\nsBaseHashtable.h(311) : error C2558: class
'nsAutoPtr<T>' : no copy constructor available or copy con
structor is declared 'explicit'
        with
        [
            T=CategoryNode
        ]
        c:/Mozilla\mozilla\xpcom\components\../base\nsAutoPtr.h(132) : while
compiling class-template member function 'nsBaseHasht
ableET<KeyClass,DataType>::nsBaseHashtableET(const
nsBaseHashtableET<KeyClass,DataType> &)'
        with
        [
            KeyClass=nsDepCharHashKey,
            DataType=nsAutoPtr<CategoryNode>
        ]
        c:/Mozilla\mozilla\xpcom\components\../base\nsAutoPtr.h(132) : while
compiling class-template member function 'void nsTHas
htable<EntryType>::s_CopyEntry(PLDHashTable *,const PLDHashEntryHdr
*,PLDHashEntryHdr *)'
        with
        [
            EntryType=nsBaseHashtableET<nsDepCharHashKey,nsAutoPtr<CategoryNode>>
        ]
       
c:\Mozilla\mozilla\obj-thunderbird\dist\include\string\nsBufferHandle.h(267) :
while compiling class-template member funct
ion 'nsTHashtable<EntryType>::nsTHashtable(void)'
        with
        [
            EntryType=nsBaseHashtableET<nsDepCharHashKey,nsAutoPtr<CategoryNode>>
        ]
        c:\Mozilla\mozilla\xpcom\ds\nsBaseHashtable.h(84) : see reference to
class template instantiation 'nsTHashtable<EntryType>
' being compiled
        with
        [
            EntryType=nsBaseHashtableET<nsDepCharHashKey,nsAutoPtr<CategoryNode>>
        ]
        c:/Mozilla\mozilla\xpcom\components\../ds\nsClassHashtable.h(56) : see
reference to class template instantiation 'nsBaseHa
shtable<KeyClass,DataType,UserDataType>' being compiled
        with
        [
            KeyClass=nsDepCharHashKey,
            DataType=nsAutoPtr<CategoryNode>,
            UserDataType=CategoryNode *
        ]
        c:\Mozilla\mozilla\xpcom\components\nsCategoryManager.h(155) : see
reference to class template instantiation 'nsClassHasht
able<KeyClass,T>' being compiled
        with
        [
            KeyClass=nsDepCharHashKey,
            T=CategoryNode
        ]
make[4]: *** [nsCategoryManager.obj] Error 2

Comment 1

[Doug Turner (:dougt)]

You are doing something wrong.  I build with msvc++ .net 2003 without any problem.

Comment 2

[Hamish Mackenzie]

Looks like a bug to me.  nsAutoPtr has a copy constructer but it takes a non
const reference (as it is going to modify the source).  Which I imagine is correct.

nsBaseHashtableET has been instanciated with the
DataType=nsAutoPtr<CategoryNode>.  It's copy constructor is failing to compile
as it is unable to copy the mData member because it has only a const reference
to the source.

Possible solutions
1) Change nsBaseHashtable::nsBaseHashtable to take nonconst ref (rembering this
will make mData null in the source).
2) Make a deep copy of mData in nsBaseHashtable::nsBaseHashtable
3) Make nsBaseHashtable noncopyable
4) Change other classes so nsBaseHashtable::DataType can be nonowning pointer

I am not familiar enough with the code in too say which answer is right.

Comment 3

[Benjamin Smedberg]

hrm, this function is never called, it merely needs to exist to make other
things compile correctly...

Comment 4

[Ali Ebrahim]

I'm seeing this problem too. I'm not familiar enough with C++ to provide a
patch, but it would be great if someone else could.

Doug, can you provide instructions somewhere for how you're building on MSVC++
.NET 2003?

Comment 5

[Benjamin Smedberg]

Attachment: de-constipate nsBaseHashtableET copy constructor

Comment 6

[Benjamin Smedberg]

Comment on attachment 129286 [details] [diff] [review]
de-constipate nsBaseHashtableET copy constructor

This should work, but I don't have a VC7.1 to test on... swalker, please
compile this to test and mark review if it works.

Comment 7

[Stephen Walker]

Comment on attachment 129286 [details] [diff] [review]
de-constipate nsBaseHashtableET copy constructor

Indeed, it does work.

Comment 8

[Benjamin Smedberg]

Comment on attachment 129286 [details] [diff] [review]
de-constipate nsBaseHashtableET copy constructor

This patch is essentially no-risk, assuming it compiles on all the revelant
platforms.

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 129286 [details] [diff] [review]
de-constipate nsBaseHashtableET copy constructor

please re-request approval when you've got the full reviews.

also, please let us know what platforms this has been tested on. thanks.

Comment 10

[Scott Collins]

Comment on attachment 129286 [details] [diff] [review]
de-constipate nsBaseHashtableET copy constructor

sr=scc

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 129286 [details] [diff] [review]
de-constipate nsBaseHashtableET copy constructor

a=asa (on behalf of drivers) for checkin to 1.5beta.

Comment 12

[Ali Ebrahim]

Any word on when this will be checked into the trunk?

Comment 13

[Benjamin Smedberg]

fixed on trunk