Fix NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS not to take obstype_ parameter

REOPENED
Unassigned

Status

()

Core
XPCOM
P5
enhancement
REOPENED
11 years ago
5 months ago

People

(Reporter: smaug, Unassigned)

Tracking

({helpwanted})

Trunk
x86
All
helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=c++] taken 2013-03-14)

Attachments

(1 attachment)

Updated

5 years ago
Keywords: helpwanted
Priority: -- → P5
Whiteboard: [mentor=bsmedberg][lang=c++]

Comment 1

5 years ago
Sir i want to take this bug i am well acquainted with the knowledge of c/c++ and would give my best effort to solve this bug.

Comment 2

5 years ago
Excellent. I *think* that a typedef as mentioned in that bug will basically make the parameter unnecessary. Please let me know if you have any questions.
Assignee: nobody → rahulgandhi38
Whiteboard: [mentor=bsmedberg][lang=c++] → [mentor=bsmedberg][lang=c++] taken 2013-03-14

Comment 3

5 years ago
So we have to Fix NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS not to take obstype_ parameter. So From where to begin?


(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Excellent. I *think* that a typedef as mentioned in that bug will basically
> make the parameter unnecessary. Please let me know if you have any questions.

Comment 4

5 years ago
Created attachment 726632 [details] [diff] [review]
Fix NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS not to take obstype_ parameter

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME

Comment 5

5 years ago
MXR (http://mxr.mozilla.org/mozilla-central/search?string=ns_observer_array) suggests this isn't resolved. Rahul's working on some other bugs right now.
Assignee: rahulgandhi38 → nobody
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
https://bugzilla.mozilla.org/show_bug.cgi?id=380674#c4 The idea is using array_::elem_type, but when I tried to add |typedef T elem_type| in nsTObserverArray, I could not figure out where should I put |typename T| to get compiled. Any help please? Thank you.
After spending whole night looking up C++ rules, I still couldn't find a solution to solve this. So bsmedberg, could you please figure it out? Or is it possible to get rid of |obstype_| parameter? Thank you.
Flags: needinfo?(benjamin)
As mentioned on IRC, you should be able to do:

  decltype(array_)::ObserverType obs_;

Make sure you include "mozilla/Types.h".

Also, adding |typename| is only needed when you do something like this:

 typedef typename T::InnerType ObserverType;

In order to differentiate between an inner type and a static member of T, for example.
Flags: needinfo?(benjamin)
push to try https://tbpl.mozilla.org/?tree=Try&rev=23b7cfcef2cc
Thank you :)
Assignee: nobody → pylaurent1314
Hi Reuben, the patch couldn't be built under "Windows XP Opt", "B2G Desktop Windows Opt", "B2G ICS Emulator Opt". Any help? The patch is here https://pastebin.mozilla.org/4112500
Flags: needinfo?(reuben.bmo)

Comment 11

4 years ago
Windows error: c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\dist\include\nsNodeUtils.h(119) : error C2039: 'elem_type' : is not a member of '`global namespace''

From this code: https://hg.mozilla.org/try/annotate/23b7cfcef2cc/content/base/src/nsNodeUtils.h#l119
through this macro: https://hg.mozilla.org/try/rev/23b7cfcef2cc#l3.94

Which basically looks like MSVC is for some reason expanding "decltype()" to the empty string.

I cannot explain this yet...
Flags: needinfo?(reuben.bmo)
Attachment #726632 - Attachment is patch: true
(In reply to Peiyong Lin[:lpy](UTC+8) from comment #10)
> Hi Reuben, the patch couldn't be built under "Windows XP Opt", "B2G Desktop
> Windows Opt", "B2G ICS Emulator Opt". Any help? The patch is here
> https://pastebin.mozilla.org/4112500

The pastebin is gone, can you attach the patch to the bug here?
(Assignee)

Updated

4 years ago
Mentor: benjamin@smedbergs.us
Whiteboard: [mentor=bsmedberg][lang=c++] taken 2013-03-14 → [lang=c++] taken 2013-03-14

Updated

5 months ago
Assignee: linpyong → nobody
Mentor: benjamin@smedbergs.us
You need to log in before you can comment on or make changes to this bug.