get rid of nsVoidArray from /mailnews

RESOLVED FIXED in Thunderbird 41.0

Status

defect
--
blocker
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: sgautherie, Assigned: ssitter)

Tracking

(Blocks 2 bugs, {dogfood})

Trunk
Thunderbird 41.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird39 unaffected, thunderbird40 fixed, thunderbird41 fixed)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
"Found 122 matching lines in 39 files"

Comment 1

7 years ago
Can you explain why it should be removed and what it needs to be changed to?
(Reporter)

Comment 2

7 years ago
See bug 485693 (and bug 474369)...

Updated

4 years ago
Blocks: 819090

Updated

4 years ago
Severity: normal → blocker

Comment 3

4 years ago
"Found 119 matching lines in 34 files"
Comment hidden (obsolete)
Comment hidden (obsolete)

Comment 6

4 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).
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

4 years ago
Thanks, hg up 617dbce26726 brings the missing nsVoidArray.h back, let's see whether the rest compiles.

Comment 9

4 years ago
Compiled ;-)
Keywords: dogfood
I was hoping one of you would be motivated to fix this. Anyone working on it, or willing to work on it?
(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

Updated

4 years ago
Duplicate of this bug: 1163323
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

4 years ago
Actually, it was removed in bug 819090.

How is the progress? We are really hampered at the moment.
(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.
Posted patch wip (obsolete) — Splinter Review
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 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.
Posted patch wip still (obsolete) — Splinter Review
Attachment #8604506 - Attachment is obsolete: true
Attachment #8604506 - Flags: feedback?(rkent)
Attachment #8604581 - Flags: feedback?(rkent)

Comment 52

4 years ago
Posted file errlog_8604581.txt
Error log for attachment 8604581 [details] [diff] [review] on SM-Trunk Linux x86_64.
(Assignee)

Comment 53

4 years ago
This patch compiles on my Windows system, maybe you can check it on your systems.
Stefan: I'll check your recent patch.
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

4 years ago
With attachment 8604875 [details] [diff] [review] building SM-Trunk Linux x86_64 succeeds. I am now using this SM.
Attachment #8604581 - Attachment is obsolete: true
Attachment #8604581 - Flags: feedback?(rkent)
Assignee: ewong → ssitter
(Assignee)

Comment 57

4 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 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+
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 on attachment 8604875 [details] [diff] [review]
Replace nsVoidArray with nsTArray, part 1

[Approval Request Comment]

Build busted.
Attachment #8604875 - Flags: approval-comm-aurora?
(Assignee)

Comment 61

4 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.
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

4 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

4 years ago
Attachment #8604875 - Attachment description: WIP → Replace nsVoidArray with nsTArray, part 1
(Assignee)

Comment 64

4 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 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

4 years ago
Keywords: checkin-needed

Comment 67

4 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

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
Comment on attachment 8605427 [details] [diff] [review]
Replace nsVoidArray with nsTArray, part 2

[Triage Comment]
Attachment #8605427 - Flags: approval-comm-aurora+
(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.
(Reporter)

Updated

4 years ago
Blocks: 1164839
You need to log in before you can comment on or make changes to this bug.