1332333 - rm EffectlesslyLookupProperty

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

This function used to have a lot  more code in it (for DOM proxies), but now it just forwards to LookupPropertyPure and we should just call LookupPropertyPure directly.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: 0001-Bug-1332333-Remove-uses-of-EffectlesslyLookupPropert.patch

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: 0002-Bug-1332333-Remove-EffectlessLookupProperty.-r-jande.patch

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8830368 [details] [diff] [review]
0001-Bug-1332333-Remove-uses-of-EffectlesslyLookupPropert.patch

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

Looks good! As mentioned below you have to rebase this though.

For these patches it doesn't matter, but most people post patches with 8 lines of context. I use Mercurial so I have these settings in my .hgrc file:

[diff]
git = 1
showfunc = true
unified = 8

::: js/src/jit/BaselineIC.cpp
@@ +2728,5 @@
>  
>      Rooted<PropertyResult> prop(cx);
>      RootedObject holder(cx);
> +    if (!LookupPropertyPure(cx, obj, id, holder.address(), prop.address()))
> +        return true;

Sorry, I landed a patch on mozilla-inbound earlier today that removes this code, so if you rebase you no longer need this change.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8830369 [details] [diff] [review]
0002-Bug-1332333-Remove-EffectlessLookupProperty.-r-jande.patch

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

Thanks for removing the comments too!

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: 0001-remove-effectlessly-lookup-property.patch

Rebased patch to m-i/tip. Forwarding r+.

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Remove EffectlessLookupProperty

From c826868fe04b8784b79331d12ee6edb37ec166f3 Mon Sep 17 00:00:00 2001

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: 0001-Bug-1332333-Remove-uses-of-EffectlesslyLookupPropert.patch

Forwarding r+. Formatted patch and rebased on inbound/tip.

Comment 8

[Ted Campbell [:tcampbell]]

Attachment: 0002-Bug-1332333-Remove-EffectlessLookupProperty.-r-jande.patch

Forward r+. Fix patch format.

Comment 9

[Ted Campbell [:tcampbell]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9085b2ae90e34c777d3d2ff1b55a4aa28ceb164

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7493fe7b336d
Remove uses of EffectlesslyLookupProperty. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bf2d6dea785
Remove EffectlessLookupProperty. r=jandem

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7493fe7b336d
https://hg.mozilla.org/mozilla-central/rev/8bf2d6dea785