569114 - TM: don't use tinyids in jsxml.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

It leaks all over the place. XMLNamespace[-2] and such. Also, remove settings caching code for reduced complexity and fewer setters/getters. We don't care that much about performance here.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 448256 [details] [diff] [review]
patch

>+DEFINE_GETTER(QName,NameURI,*vp = (*vp == JSVAL_VOID) ? JSVAL_NULL : *vp)

Should be, `if (JSVAL_IS_VOID(*vp)) *vp = JSVAL_NULL;`

Comment 3

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/4cf7355a09d4

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #3)
> http://hg.mozilla.org/tracemonkey/rev/4cf7355a09d4

I object to abusing ?: for if-then. All the lines like this one:

+              *vp = (obj->getClass() == &js_QNameClass.base) ? obj->getQNameLocalName() : *vp)

should use

+              if (obj->getClass() == &js_QNameClass.base)
+                  *vp = obj->getQNameLocalName();

instead.

Macros taking "code" formals that must be statements should not wrap in unnecessary braces and add ; at the end -- make the caller pass the whole statement in. Otherwise there's at least a warning hazard for any future caller who passes in a trailing ; and thus makes ;;.

rs=me on followup to fix this.

/be

Comment 5

[Robert Sayre]

This test has been failing on the TM tinderbox for three days:

REFTEST TEST-UNEXPECTED-PASS | file:///builds/slave/tracemonkey-linux-opt-unittest-jsreftest/build/jsreftest/tests/jsreftest.html?test=e4x/Expressions/11.1.4-04.js | Section 1 of test - 11.1.4 - XML Initializer - Comment hiding parsing/scanning item 1

seems like it might be a good thing, but check it out.

Comment 6

[Andreas Gal :gal]

host-5-104:tests gal$ python jstests.py ../Darwin_DBG.OBJ/js 11.1.4-04.js
[   1|   0|   0] 100% ===============================================>|    0.1s
PASS
host-5-104:tests gal$ 

I saw this yesterday and tried to reproduce but no luck. I wonder whats up there.

Comment 7

[Andreas Gal :gal]

Maybe the config handling I changed somehow affects this and the browser does some stuff to the xml config?

Comment 8

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/4cf7355a09d4

Update jstests.list to get rid of some more orange: remove the "fails-if" tag from a test that no longer fails. My best guess is that the seemingly trivial simplification in bug 569114 fixed an actual browser-only correctness bug. Amazing. shame=andreas, rs=Waldo, no bug#.

Comment 9

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/4cf7355a09d4