775289 - Codegen: Remove generateNativeAccessors branching.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Eric Faust [:efaust]]

We are now pretty comitted to using native accessors. We should be able to rip out the code that generates PropertyOps.

Comment 1

[Eric Faust [:efaust]]

Attachment: Patch

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

Comment 3

[Eric Faust [:efaust]]

Attachment: Best to remove it all the way.

Comment 4

[Eric Faust [:efaust]]

Attachment: Codegen.py before centralization

Codegen.py in light of the patch above.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Eric Faust [:efaust]]

Right, the only question was whether to cut CGGetterSetterCall. Since it holds basically no purpose, I'll just strike it.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Well, it commons up some details of initing CGPerSignatureCall.  But yeah, nixing it is fine.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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?

Comment 9

[Eric Faust [:efaust]]

Attachment: Remove GetterSetterCall as well

Comment 10

[Peter Van der Beken [:peterv]]

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.

Comment 11

[Eric Faust [:efaust]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/532d31e06fb8

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/532d31e06fb8