rm EffectlesslyLookupProperty

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jandem, Assigned: tcampbell)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

2 years ago
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.
(Assignee)

Updated

2 years ago
Assignee: nobody → tcampbell
(Assignee)

Comment 1

2 years ago
Created attachment 8830368 [details] [diff] [review]
0001-Bug-1332333-Remove-uses-of-EffectlesslyLookupPropert.patch
(Assignee)

Comment 2

2 years ago
Created attachment 8830369 [details] [diff] [review]
0002-Bug-1332333-Remove-EffectlessLookupProperty.-r-jande.patch
(Assignee)

Updated

2 years ago
Attachment #8830368 - Flags: review?(jdemooij)
(Assignee)

Updated

2 years ago
Attachment #8830369 - Flags: review?(jdemooij)
(Reporter)

Comment 3

2 years ago
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.
Attachment #8830368 - Flags: review?(jdemooij) → review+
(Reporter)

Comment 4

2 years ago
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!
Attachment #8830369 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 5

2 years ago
Created attachment 8830408 [details] [diff] [review]
0001-remove-effectlessly-lookup-property.patch

Rebased patch to m-i/tip. Forwarding r+.
Attachment #8830368 - Attachment is obsolete: true
Attachment #8830408 - Flags: review+
(Assignee)

Updated

2 years ago
Attachment #8830408 - Attachment description: remove-effectlessly-lookup-property.patch → 0001-remove-effectlessly-lookup-property.patch
Attachment #8830408 - Attachment filename: remove-effectlessly-lookup-property.patch → 0001-remove-effectlessly-lookup-property.patch
(Assignee)

Comment 6

2 years ago
Created attachment 8830467 [details] [diff] [review]
Remove EffectlessLookupProperty

From c826868fe04b8784b79331d12ee6edb37ec166f3 Mon Sep 17 00:00:00 2001
(Assignee)

Updated

2 years ago
Attachment #8830467 - Attachment is obsolete: true
(Assignee)

Comment 7

2 years ago
Created attachment 8830547 [details] [diff] [review]
0001-Bug-1332333-Remove-uses-of-EffectlesslyLookupPropert.patch

Forwarding r+. Formatted patch and rebased on inbound/tip.
Attachment #8830408 - Attachment is obsolete: true
Attachment #8830547 - Flags: review+
(Assignee)

Comment 8

2 years ago
Created attachment 8830550 [details] [diff] [review]
0002-Bug-1332333-Remove-EffectlessLookupProperty.-r-jande.patch

Forward r+. Fix patch format.
Attachment #8830369 - Attachment is obsolete: true
Attachment #8830550 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 10

2 years ago
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7493fe7b336d
https://hg.mozilla.org/mozilla-central/rev/8bf2d6dea785
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.