Closed
Bug 775289
Opened 12 years ago
Closed 12 years ago
Codegen: Remove generateNativeAccessors branching.
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: efaust, Assigned: efaust)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
We are now pretty comitted to using native accessors. We should be able to rip out the code that generates PropertyOps.
Updated•12 years ago
|
Blocks: ParisBindings
Assignee | ||
Comment 1•12 years ago
|
||
Applies on top of patches in bug 773548.
Can we do better with CG{Getter,Setter,GetterSetter}Call? It feels like they should be able to go away, but I didn't want to remove the safety from getter != setter.
Assignee | ||
Updated•12 years ago
|
Attachment #644085 -
Attachment is patch: true
Attachment #644085 -
Flags: review?(peterv)
Comment 2•12 years ago
|
||
Comment on attachment 644085 [details] [diff] [review]
Patch
>+++ b/dom/bindings/Codegen.py
>@@ -949,20 +949,18 @@ class AttrDefiner(PropertyDefiner):
>- elif attr.readonly:
>+ if attr.readonly:
> return "JSPROP_READONLY | " + flags
Shouldn't the attr.readonly branch go away altogether? Note that it was not used if generateNativeAccessors, because JSPROP_READONLY is nonsensical for an accessor property, iirc.
The rest looks fine. I'm not sure yet what to do with CG*Call; want to attach your Codegen.py as it looks after these changes?
In addition to the changes in this diff, you can also remove the following:
* The setting of Codegen.generateNativeAccessors in GlobalGen.py in main()
* The useJSOPAccessors command-line option handling in the same function
* Same two things in BindingGen.py
* The various mentions of ACCESSOR_OPT in the Makefile.in
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #644085 -
Attachment is obsolete: true
Attachment #644085 -
Flags: review?(peterv)
Attachment #644180 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #644180 -
Flags: review?
Assignee | ||
Comment 4•12 years ago
|
||
Codegen.py in light of the patch above.
Updated•12 years ago
|
Attachment #644181 -
Attachment mime type: text/x-python-script → text/plain
Comment 5•12 years ago
|
||
So I think you can nix CGGetterSetterCall and just have getter and setter directly inherit from CGPerSignatureCall if you wanted to. Or you could just leave it as-is.
You can't really common together CGGetterCall and CGSetterCall easily because they actually do different things in various ways.
Assignee | ||
Comment 6•12 years ago
|
||
Right, the only question was whether to cut CGGetterSetterCall. Since it holds basically no purpose, I'll just strike it.
Comment 7•12 years ago
|
||
Well, it commons up some details of initing CGPerSignatureCall. But yeah, nixing it is fine.
Comment 8•12 years ago
|
||
Comment on attachment 644180 [details] [diff] [review]
Best to remove it all the way.
Review of attachment 644180 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/bindings/Codegen.py
@@ +3173,3 @@
> def generate_code(self):
> + argv = ("JS::Value* argv = JS_ARGV(cx, vp);\n"
> + "jsval undef = JS::UndefinedValue();\n"
Make this JS::Value while you're here?
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #644180 -
Attachment is obsolete: true
Attachment #644545 -
Flags: review?(peterv)
Comment 10•12 years ago
|
||
Comment on attachment 644545 [details] [diff] [review]
Remove GetterSetterCall as well
Review of attachment 644545 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/bindings/Codegen.py
@@ +3171,5 @@
> argv +
> ("if (!specialized_set_%s(cx, obj, self, argv)) {\n"
> " return false;\n"
> "}\n" % self.attr.identifier.name) +
> retval +
Just replace the variables with their values here.
Attachment #644545 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 11•12 years ago
|
||
Comment 12•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•