Closed Bug 605200 Opened 9 years ago Closed 9 years ago

TM: names not bound by XML filters

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bhackett, Assigned: bhackett)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Names inside an XML filter are compiled to bytecode in the same way as names outside the filter.  JSOP_FILTER/ENDFILTER are implemented using EnterWith/LeaveWith, so it seems that a filter should bind names in the XML object as for a 'with'.

this:

function foo() {
  var x = <a><name>what</name></a>;
  print(x.(name == "what"));
}
foo();

prints:

<a>
  <name>what</name>
</a>

this:

function foo() {
  var name = 2;
  var x = <a><name>what</name></a>;
  print(x.(name == "what"));
}
foo();

prints nothing.  dis(foo) reveals the name access in the filter is a GETLOCAL.  Also, in the first example the name access is a GETGNAME rather than a GETNAME.  This works for the interpreter because GETGNAME and GETNAME are treated identically, but would break in JM (JM does not currently compile XML filters).
Attached patch patchSplinter Review
This changes filter expressions to behave like 'with' statements wrt handling names within the filter.  The current behavior breaks inference on filter expressions, including several jstests when -a is used.  Who should review this?
Assignee: general → bhackett1024
Waldo or Brendan.
Attachment #517368 - Flags: review?(brendan)
Comment on attachment 517368 [details] [diff] [review]
patch

..12345678901234567890123456789012345678901234567890123456789012345678901234567890

>+            /* Treat filters like with statements for the purpose of deoptimizing name accesses. */

Nits: "as 'with'", not "like with", and wrap into major coment before column 80, or (better) shorten a bit via "for name deoptimization."

Great to see this, thanks for fixing (was a bug on my list, reported by jorendorff: bug 627480).

/be
Attachment #517368 - Flags: review?(brendan) → review+
http://hg.mozilla.org/tracemonkey/rev/b1f8f4f84662
Whiteboard: fixed-in-tracemonkey
Backed out and relanded a simpler patch.  The first patch deoptimized all name accesses in the 'with', including those bound by an inner block, breaking some block chain optimizations.

http://hg.mozilla.org/tracemonkey/rev/34c628db78cb
Nice, that landed patch is much smaller and (as it happens) correct. I should have caught the over-deoptimization in the previous one -- sorry!

/be
http://hg.mozilla.org/mozilla-central/rev/34c628db78cb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.