Closed
Bug 397929
Opened 14 years ago
Closed 13 years ago
Allow changing the C++ method names generated for attributes or methods to avoid naming conflicts
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta4
People
(Reporter: benjamin, Assigned: Waldo)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 2 obsolete files)
7.45 KB,
patch
|
benjamin
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
9.39 KB,
patch
|
Waldo
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
This is for moz2: windows.h ends up redefining a bunch of very common tokens such as GetMessage ERROR CompareString GetClassName Because MMgc.h ends up #including <windows.h> we end up redefining these a lot in places where we previously didn't. Some of these places can be fixed with a simple #undef, but GetMessage in particular can't: GetMessage is used in ATL headers and must not be undefined. So I've hacked XPIDL to emit a different binary method name for that particular case. I didn't rename the signature in nsIConsoleMessage because 1) that's hard to get right with all the JS callers and 2) there appear to be places which expect JS exception .message to match the .message of nsIScriptError->nsIConsoleMessage
Attachment #282734 -
Flags: superreview?(shaver)
Attachment #282734 -
Flags: review?(shaver)
Assignee | ||
Comment 1•13 years ago
|
||
I think I may have a principled fix; it works on my one testcase so far. I'm rebuilding now to check...
Assignee: benjamin → jwalden+bmo
Assignee | ||
Comment 2•13 years ago
|
||
Should be simple enough to understand...just add binaryname(Name) to the bracketed attributes for a method or attribute, and then you either use Class::Name or [GS]etName to define the method. This patch works for me with postMessage, and it worked by visual inspection of the generated header on the following attribute in an IDL interface: [binaryname(MessageMoz)] attribute AString message;
Attachment #282734 -
Attachment is obsolete: true
Attachment #301449 -
Flags: review?(benjamin)
Attachment #282734 -
Flags: superreview?(shaver)
Attachment #282734 -
Flags: review?(shaver)
Assignee | ||
Comment 3•13 years ago
|
||
I don't see a reason to bump the iid since the patch preserves binary compatibility.
Blocks: postMessage
Status: NEW → ASSIGNED
Updated•13 years ago
|
Version: unspecified → Trunk
Reporter | ||
Comment 4•13 years ago
|
||
Comment on attachment 301449 [details] [diff] [review] Patch Beautiful.
Attachment #301449 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 5•13 years ago
|
||
Comment on attachment 301449 [details] [diff] [review] Patch (XPCOM doesn't require sr, just in case you, Mr. Anonymous Approval-Granter, didn't know. :-) )
Attachment #301449 -
Flags: approval1.9?
Updated•13 years ago
|
Attachment #301449 -
Flags: approval1.9? → approval1.9+
Reporter | ||
Updated•13 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•13 years ago
|
||
I'll pick this up later today, most likely tonight.
Keywords: checkin-needed
Assignee | ||
Comment 7•13 years ago
|
||
I documented this in: http://www.mozilla.org/scriptable/xpidl/idl-authors-guide/rules.html I think we should leave renaming individual names like className, message, etc. to followup bugs, for the most part, but I'd like to add this to at least one attribute so it's demonstrated that the attribute stuff works. Patch for nsIConsoleMessage.message to do this in a sec...
Component: XPCOM → xpidl
Keywords: dev-doc-complete
QA Contact: xpcom → xpidl
Summary: GetMessage redefined by windows.h used in nsIConsoleMessage, and other redefinitions that hurt → Allow changing the C++ method names generated for attributes or methods to avoid naming conflicts
Target Milestone: --- → mozilla1.9beta4
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #302036 -
Flags: review?(benjamin)
Reporter | ||
Updated•13 years ago
|
Attachment #302036 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 9•13 years ago
|
||
For the record, I'm trying to fight tryserver into verifying that Windows will run with this patch, particularly with the #undef GetMessage hunk of it. It's been slow going due to lots of recent tryserver bustage (I'm on my second bug report to get the Windows one working). I'll land once I get verification that nothing else depends on that #undef-inition.
Assignee | ||
Comment 10•13 years ago
|
||
Attachment #302036 -
Attachment is obsolete: true
Attachment #302735 -
Flags: review+
Attachment #302735 -
Flags: approval1.9?
Updated•13 years ago
|
Attachment #302735 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 11•13 years ago
|
||
In on trunk. Now that both methods and attributes are being used here, I'm going to mark this FIXED; further cleanup of Windows's mess can go in other bugs.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 416951
Comment 12•13 years ago
|
||
How do I know if I need to put this [binaryname(MessageMoz)] thing in a .idl or not? For example, webservices has a message attribute in nsISOAPMessage.idl but there are no undef GetMessage's anywhere in the webservices code. And it returns a nsIDOMDocument, not a string.
Reporter | ||
Comment 13•13 years ago
|
||
Until we start having compiler errors, don't worry about it.
Comment 14•13 years ago
|
||
(In reply to comment #13) > Until we start having compiler errors, don't worry about it. > Well, reed changed nsSOAPMessage in webservices to have GetMessageMoz which caused compile errors because he missed some spots. So I was trying determine if his change was even needed. Since webservices compiled fine without his change, I'll probably back it out. But I will still need to fix webservices proxy because has a class that extends nsIException which did change to use GetMessageMoz. I'll open a bug on webservices to make these changes.
Comment 15•13 years ago
|
||
(In reply to comment #14) > Well, reed changed nsSOAPMessage in webservices to have GetMessageMoz which > caused compile errors because he missed some spots. So I was trying determine > if his change was even needed. Since webservices compiled fine without his > change, I'll probably back it out. This was why I changed it. <Ubulette> [reed], trunk is broken http://paste.ubuntu.com/4517/ ...I make change... <Ubulette> [reed], still broken <Ubulette> nsSOAPMessage.cpp:68: error: no 'nsresult nsSOAPMessage::GetMessageMoz(nsIDOMDocument**)' member function declared in class 'nsSOAPMessage' <Ubulette> before, it was <Ubulette> nsSOAPException.cpp:63: error: no 'nsresult nsSOAPException::GetMessage(char**)' member function declared in class 'nsSOAPException' <Ubulette> http://paste.ubuntu.com/4525/ So, webservices does need some change somewhere, it seems.
Comment 16•13 years ago
|
||
Thanks Reed, that helps. I see why nsSOAPException would give an error, but not nsSOAPMessage. Maybe it is a platform thing. I guess I'll leave nsSOAPMessage alone and just add what I need to get my windows machine building again.
![]() |
||
Comment 17•13 years ago
|
||
So... Over here, I'm not seeing any GetMessageMoz in the generated nsISoapMessage.h. On the other hand, the C++ was changed to use GetMessageMoz, so the whole thing can't possibly compile... Anyone care to shed some light on how the heck this is supposed to work?
![]() |
||
Comment 18•13 years ago
|
||
Hmm. Looks like I'd somehow managed to pick up the C++ changes but not the idl changes... repulling tip from toplevel made it work...
You need to log in
before you can comment on or make changes to this bug.
Description
•