Closed
Bug 777770
Opened 12 years ago
Closed 9 years ago
get rid of nsVoidArray from /mailnews
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(thunderbird39 unaffected, thunderbird40 fixed, thunderbird41 fixed)
RESOLVED
FIXED
Thunderbird 41.0
Tracking | Status | |
---|---|---|
thunderbird39 | --- | unaffected |
thunderbird40 | --- | fixed |
thunderbird41 | --- | fixed |
People
(Reporter: sgautherie, Assigned: ssitter)
References
(Blocks 2 open bugs, )
Details
(Keywords: dogfood)
Attachments
(3 files, 2 obsolete files)
5.34 KB,
text/plain
|
Details | |
127.94 KB,
patch
|
rkent
:
review+
rkent
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
44.48 KB,
patch
|
rkent
:
review+
rkent
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
"Found 122 matching lines in 39 files"
Can you explain why it should be removed and what it needs to be changed to?
Reporter | ||
Comment 2•12 years ago
|
||
See bug 485693 (and bug 474369)...
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Comment 6•9 years ago
|
||
Is there a plan to fix this? Does someone know how to revert back to a working rev, on #maildev they said: cd mozilla && hg up <rev> (I don't know what rev works though).
Comment 7•9 years ago
|
||
If you go to treeherder https://treeherder.mozilla.org/#/jobs?repo=comm-central and click n the green B from a recently completed job, you can see which mozilla rev was used successfully there. I see 617dbce26726
Comment 8•9 years ago
|
||
Thanks, hg up 617dbce26726 brings the missing nsVoidArray.h back, let's see whether the rest compiles.
Comment 9•9 years ago
|
||
Compiled ;-)
Comment 10•9 years ago
|
||
I was hoping one of you would be motivated to fix this. Anyone working on it, or willing to work on it?
Comment 11•9 years ago
|
||
(In reply to Kent James (:rkent) from comment #10) > I was hoping one of you would be motivated to fix this. Anyone working on > it, or willing to work on it? Working on it.
Assignee: nobody → ewong
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 47•9 years ago
|
||
Actually, it was removed in bug 819090. How is the progress? We are really hampered at the moment.
Comment 48•9 years ago
|
||
(In reply to Jorg K from comment #47) > Actually, it was removed in bug 819090. > > How is the progress? We are really hampered at the moment. I'm sorry for the slow going. Currently working on it.
Comment 49•9 years ago
|
||
Current WIP. It doesn't build as I'm getting the following: 1:21.14 Warning: C4180 in e:\mozstuff\mozprgs\trunk\objdir\dist\include\nsTArray.h: qualifier applied to function type has no meaning; ignored 1:21.14 e:\mozstuff\mozprgs\trunk\objdir\dist\include\nsTArray.h(1647) : warning C4180: qualifier applied to function type has no meaning; ignored 1:21.14 e:\mozstuff\mozprgs\trunk\objdir\dist\include\nsTArray.h(1660) : see reference to function template instantiation 'int nsTArray_Impl<E,nsTArrayInfallibleAllocator>::Compare<Comparator>(const void *,const void *,void *)' being compiled 1:21.14 with 1:21.14 [ 1:21.14 E=void * 1:21.14 , Comparator=int (const void *,const void *,void *) 1:21.14 ] 1:21.14 e:\mozstuff\mozprgs\trunk\objdir\dist\include\nsTArray.h(1660) : see reference to function template instantiation 'int nsTArray_Impl<E,nsTArrayInfallibleAllocator>::Compare<Comparator>(const void *,const void *,void *)' being compiled 1:21.14 with 1:21.14 [ 1:21.14 E=void * 1:21.14 , Comparator=int (const void *,const void *,void *) 1:21.14 ] 1:21.14 e:/mozstuff/mozprgs/trunk/mailnews/addrbook/src/nsAbView.cpp(751) : see reference to function template instantiation 'void nsTArray_Impl<E,nsTArrayInfallibleAllocator>::Sort<int(const void *,const void *,void *)>(Comparator (__cdecl &))' being compiled 1:21.14 with 1:21.14 [ 1:21.14 E=void * 1:21.14 , Comparator=int (const void *,const void *,void *) 1:21.14 ] 1:21.14 e:/mozstuff/mozprgs/trunk/mailnews/addrbook/src/nsAbView.cpp(751) : see reference to function template instantiation 'void nsTArray_Impl<E,nsTArrayInfallibleAllocator>::Sort<int(const void *,const void *,void *)>(Comparator (__cdecl &))' being compiled 1:21.14 with 1:21.14 [ 1:21.14 E=void * 1:21.14 , Comparator=int (const void *,const void *,void *) 1:21.14 ] 1:21.14 e:\mozstuff\mozprgs\trunk\objdir\dist\include\nsTArray.h(1650) : error C2227: left of '->LessThan' must point to class/struct/union/generic type 1:21.14 type is 'int (__cdecl *)(const void *,const void *,void *)' 1:21.14 e:\mozstuff\mozprgs\trunk\objdir\dist\include\nsTArray.h(1650) : error C2227: left of '->Equals' must point to class/struct/union/generic type 1:21.14 type is 'int (__cdecl *)(const void *,const void *,void *)' 1:21.14 1:21.15 In the directory /e/mozstuff/mozprgs/trunk/objdir/mailnews/addrbook/src 1:21.15 The following command failed to execute properly: 1:21.15 e:/mozstuff/mozprgs/trunk/objdir/_virtualenv/Scripts/python.exe -m mozbuild.action.cl cl -FonsAbView.obj -c -I../../../dist/stl_wrappers -DMOZ_LDAP_XPCOM -DNOMINMAX -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DAB_CD=en-US -DNO_NSPR_10_SUPPORT -Ie:/mozstuff/mozprgs/trunk/mailnews/addrbook/src -I. -I../../../dist/include -Ie:/mozstuff/mozprgs/trunk/objdir/dist/include/nspr -Ie:/mozstuff/mozprgs/trunk/objdir/dist/include/nss -MD -FI ../../../dist/include/mozilla-config.h -DMOZILLA_CLIENT -TP -nologo -D_HAS_EXCEPTIONS=0 -W3 -Gy -arch:IA32 -FS -wd4251 -wd4244 -wd4267 -wd4345 -wd4351 -wd4482 -wd4800 -wd4819 -we4553 -GR- -DNDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -O1 -Oi -Oy -Fdgenerated.pdb e:/mozstuff/mozprgs/trunk/mailnews/addrbook/src/nsAbView.cpp 1:21.16 e:/mozstuff/mozprgs/trunk/mozilla/config/rules.mk:932: recipe for target 'nsAbView.obj' failed 1:21.16 mozmake.EXE[4]: *** [nsAbView.obj] Error 1 1:21.16 e:/mozstuff/mozprgs/trunk/mozilla/config/recurse.mk:70: recipe for target 'mailnews/addrbook/src/target' failed 1:21.16 mozmake.EXE[3]: *** [mailnews/addrbook/src/target] Error 2 1:21.16 mozmake.EXE[3]: *** Waiting for unfinished jobs....
Attachment #8604506 -
Flags: feedback?(rkent)
Comment 50•9 years ago
|
||
Comment on attachment 8604506 [details] [diff] [review] wip Actually, I have managed to get past that minor bump with the help from Neil over irc. will update the patch when I can, but feedback is certainly appreciated.
Comment 51•9 years ago
|
||
Attachment #8604506 -
Attachment is obsolete: true
Attachment #8604506 -
Flags: feedback?(rkent)
Attachment #8604581 -
Flags: feedback?(rkent)
Comment 52•9 years ago
|
||
Error log for attachment 8604581 [details] [diff] [review] on SM-Trunk Linux x86_64.
Assignee | ||
Comment 53•9 years ago
|
||
This patch compiles on my Windows system, maybe you can check it on your systems.
Comment 54•9 years ago
|
||
Stefan: I'll check your recent patch.
Comment 55•9 years ago
|
||
Comment on attachment 8604875 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 1 Yes compiles for me, pushed to try https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=656db41521fb Stefan, if you are happy with your patch can you please request review, or do any needed updates?
Comment 56•9 years ago
|
||
With attachment 8604875 [details] [diff] [review] building SM-Trunk Linux x86_64 succeeds. I am now using this SM.
Updated•9 years ago
|
Attachment #8604581 -
Attachment is obsolete: true
Attachment #8604581 -
Flags: feedback?(rkent)
Updated•9 years ago
|
Assignee: ewong → ssitter
Assignee | ||
Comment 57•9 years ago
|
||
Comment on attachment 8604875 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 1 In my opinion the patch is not complete because it currently introduces several new warnings like signed/unsigned mismatch or because there are many remains of now unnecessary casts to and from void*. But maybe it is complete enough to fix the build breakage therefore review would be appreciated. Some changes like the new CardComperator I gratefully took over from Edmunds patch and other changes like in the idl I just did by comparing with existing code. It compiles but I don't know if everything works as before :)
Attachment #8604875 -
Flags: review?(rkent)
Comment 58•9 years ago
|
||
Comment on attachment 8604875 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 1 Review of attachment 8604875 [details] [diff] [review]: ----------------------------------------------------------------- Overall looks good. I think that we need to get this landed as it is blocking critical stuff. Could you file a followup bug to do any remaining cleanup? r=me with few nits fixed. ::: mailnews/addrbook/src/nsAbView.cpp @@ +683,5 @@ > + private: > + SortClosure *m_closure; > + > + public: > + void setclosure(SortClosure *closure) { m_closure = closure; }; Please call this SetClosure @@ +684,5 @@ > + SortClosure *m_closure; > + > + public: > + void setclosure(SortClosure *closure) { m_closure = closure; }; > + SortClosure * retclosure() { return m_closure; }; You never call retclosure so just remove this. ::: mailnews/base/src/nsMsgDBView.h @@ +35,5 @@ > #include "nsAutoPtr.h" > #include "nsIWeakReferenceUtils.h" > #define MESSENGER_STRING_URL "chrome://messenger/locale/messenger.properties" > > +template <class> class nsTArray; You don't need this, please delete. ::: mailnews/imap/src/nsIMAPNamespace.cpp @@ +113,5 @@ > > int nsIMAPNamespaceList::GetNumberOfNamespaces(EIMAPNamespaceType type) > { > int nodeIndex = 0, count = 0; > + for (nodeIndex=m_NamespaceList.Length()-1; nodeIndex >= 0; nodeIndex--) Nit: space around "=" and "-1" @@ +133,5 @@ > if (!ns->GetIsNamespaceFromPrefs()) > { > int nodeIndex; > // iterate backwards because we delete elements > + for (nodeIndex=m_NamespaceList.Length()-1; nodeIndex >= 0; nodeIndex--) Nit: space around "=" and "-1" (and more below) ::: mailnews/import/oexpress/nsOEScanBoxes.cpp @@ +476,2 @@ > { > pEntry = (MailboxEntry *) m_pendingChildArray.ElementAt(i); You don't need the (MailboxEntry *) @@ +579,5 @@ > } > } > > + if (m_entryArray.Length() > 0) { > + pEntry = (MailboxEntry *)m_entryArray.ElementAt(m_entryArray.Length() - 1); You don't need the (MailboxEntry *) @@ +644,5 @@ > } > } > > + if (m_entryArray.Length() > 0) { > + pEntry = (MailboxEntry *)m_entryArray.ElementAt(m_entryArray.Length() - 1); You don't need the (MailboxEntry *) @@ +660,1 @@ > pBox = (MailboxEntry *) m_entryArray.ElementAt(0); You don't need the (MailboxEntry *) @@ +708,1 @@ > pBox = (MailboxEntry *) m_entryArray.ElementAt(0); You don't need the (MailboxEntry *) @@ +767,2 @@ > for (int32_t i = 0; i < max; i++) { > MailboxEntry *pEntry = (MailboxEntry *) m_entryArray.ElementAt(i); You don't need the (MailboxEntry *)
Attachment #8604875 -
Flags: review?(rkent) → review+
Comment 59•9 years ago
|
||
I finished the nits, and pushed: https://hg.mozilla.org/comm-central/rev/d294256abf7b Sorry to rush this, but the closed tree is causing major disruption.
Comment 60•9 years ago
|
||
Comment on attachment 8604875 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 1 [Approval Request Comment] Build busted.
Attachment #8604875 -
Flags: approval-comm-aurora?
Updated•9 years ago
|
status-thunderbird39:
--- → unaffected
status-thunderbird40:
--- → affected
status-thunderbird41:
--- → fixed
Assignee | ||
Comment 61•9 years ago
|
||
Bah, I just finished creating a patch that contains the changes as requested by you + the outstanding fixes mentioned by me but now my patch contains only merge errors.
Comment 62•9 years ago
|
||
Sorry about that. I'm on a fairly tight schedule trying to get the (already late) Tb 38 finished, and I really need to get some pushes done.
Assignee | ||
Comment 63•9 years ago
|
||
If I got it right this should be the difference to the original patch to be applied on top of it.
Attachment #8605427 -
Flags: review?(rkent)
Assignee | ||
Updated•9 years ago
|
Attachment #8604875 -
Attachment description: WIP → Replace nsVoidArray with nsTArray, part 1
Assignee | ||
Comment 64•9 years ago
|
||
Or to be more precise attachment #8605427 [details] [diff] [review] should be applied on top of the modified version of attachment #8604875 [details] [diff] [review] that was pushed to https://hg.mozilla.org/comm-central/rev/d294256abf7b
Comment 65•9 years ago
|
||
Comment on attachment 8605427 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 2 Review of attachment 8605427 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #8605427 -
Flags: review?(rkent) → review+
Updated•9 years ago
|
Keywords: checkin-needed
Comment 67•9 years ago
|
||
For future reference, please update the uuid in all interfaces you touch: remote: Push rejected because the following IDL interfaces were remote: modified without changing the UUID: remote: - nsIPop3Protocol in changeset 3eb84dd9febc remote: remote: To update the UUID for all of the above interfaces and their remote: descendants, run: remote: ./mach update-uuids nsIPop3Protocol
Keywords: checkin-needed
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
Comment 68•9 years ago
|
||
Comment on attachment 8605427 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 2 [Triage Comment]
Attachment #8605427 -
Flags: approval-comm-aurora+
Comment 69•9 years ago
|
||
Comment on attachment 8604875 [details] [diff] [review] Replace nsVoidArray with nsTArray, part 1 https://hg.mozilla.org/releases/comm-aurora/rev/44917543a77b https://hg.mozilla.org/releases/comm-aurora/rev/7af2eea538ce
Attachment #8604875 -
Flags: approval-comm-aurora? → approval-comm-aurora+
Updated•9 years ago
|
Comment 70•9 years ago
|
||
(In reply to Stefan Sitter from comment #57) > Comment on attachment 8604875 [details] [diff] [review] > Replace nsVoidArray with nsTArray, part 1 > > In my opinion the patch is not complete because it currently introduces > several new warnings like signed/unsigned mismatch or because there are many > remains of now unnecessary casts to and from void*. > > But maybe it is complete enough to fix the build breakage therefore review > would be appreciated. Some changes like the new CardComperator I gratefully > took over from Edmunds patch and other changes like in the idl I just did by > comparing with existing code. It compiles but I don't know if everything > works as before :) CardComparator was Neil's idea.
You need to log in
before you can comment on or make changes to this bug.
Description
•