714079 - Remove defineGeneric

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Comment 1

[Tom S [:evilpie]]

Attachment: WIP

Comment 2

[Tom S [:evilpie]]

Attachment: WIP

real patch this time

Comment 3

[Tom S [:evilpie]]

Attachment: v1

There are some things, in the patch that are very dirty, but until we have some kind of sane API that we can call in all the weird cases, this is probably the most sane solution.

Try results should be posted here. \o/

Comment 4

[Mozilla RelEng Bot]

Try run for 54930bf1e0d3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=54930bf1e0d3
Results (out of 271 total builds):
    success: 235
    warnings: 34
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-54930bf1e0d3

Comment 5

[Tom S [:evilpie]]

Attachment: v2

So I was able to make two mistakes in one line of code.

>if (JS_DefinePropertyById(cx, obj, shape->propid(), v, getter, setter, attrs))
Yep.

Comment 6

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

Comment on attachment 585029 [details] [diff] [review]
v2

>diff --git a/js/src/jsclass.h b/js/src/jsclass.h
>--- a/js/src/jsclass.h
>+++ b/js/src/jsclass.h
>@@ -1,8 +1,9 @@
>+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
> /* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-

Ahem.

Comment 7

[Jeff Walden [:Waldo]]

Comment on attachment 585029 [details] [diff] [review]
v2

Review of attachment 585029 [details] [diff] [review]:
-----------------------------------------------------------------

I think this wants to go the defineByValue direction, along the lines that deleteByValue just went.  Note that not all jsids are Values, so use IdToValue(id) passed to defineByValue carefully (see the last comment in bug 713965).

You should make sure the browser builds with this patch; I think you need to remove some NULLs from JSCLASS_NO_OPTIONAL_MEMBERS (?, memory hazy on the name -- it's in jsclass.h, easy enough to find).

Comment 8

[Tom S [:evilpie]]

I am not going to work on this in the near future.

Comment 9

[André Bargull [:anba]]

All gone in https://hg.mozilla.org/mozilla-central/rev/f1ac9100750e, resolving as WFM.