418624 - Issues with vector used with number indexes

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P1)

Description

[Erik Tierney]

Attachment: Code showing the incorrect behavior.

Getting/setting properties of a vector with string/number indexes should fail, but currently works.  Attempts to set v["3"] or v[1.1] work, but shouldn't.

Comment 1

[Erik Tierney]

Attachment: Code showing the incorrect behavior

Add the bug file with the right content type

Comment 2

[Dan Schaffer]

bug is still present in vm, mark as defer?

Comment 3

[Erik Tierney]

So, the vector proposal says:

"NOTE   If v is a Vector then reading and writing v[3.14] or v[-3] will always fail, though reading and writing v["3.14"] or v["-3"] will succeed.

This behavior deviates from the 3rd Edition, where strings and numbers are interchangeable as property names. But that's no longer quite true in 4th Edition anyway, which has have namespaces and Name objects.

Most attempts to set or get properties that are named by numbers that are not valid array indices are probably errors, especially if the object is an Array. Most attempts to read beyond the end of an Array are probably errors. And in a number of cases, attempts to write beyond the end of an Array are probably errors too. The Vector class makes it possible to discover these errors. "


Implementing that won't be easy, as TC currently treats strings and numbers interchangably as property names.  So, by the time we are looking up something in a Vector, we have no knowledge of whether the property name was originally a string or a number.  So this would require some changes to generally property lookup, which I'm not sure we want to do at this point.

Comment 4

[Dan Schaffer]

set milestone to future, deferring bug past fp10 release.

Comment 5

[Erik Tierney]

Jeff recommended making it an error to access v[3.14], or v["3.14"] - basically anything that looks like a number.  We would be more restrictive than the ES4 proposal then, which would probably be better for future compatibility.  This would also only affect the Vector property accessor code, rather than all property accessor code.

Comment 6

[Jeff Dyer]

Also, looking at the code, the check for the error case would come after the common path taken where a uint index is provided. So this change would only incur the cost of an additional check when the property name is not a vector offset.

Agree with Erik, the proposed solution is future proof and probably doesn't get in the way of any intended programs.

(adding Lars in case he has a different opinion.)

Jd

Comment 7

[Erik Tierney]

The easiest change is probably to make it an error to access any non-indexed properties.  So accessing -1, 3.5, or "mystring" would produce an error.

Comment 8

[Erik Tierney]

Attachment: Error if accessing non-indexed properties of a Vector.

Attempt to write a non-indexed dynamic property now always fails.  Attempts to read a non-indexed dynamic property will check the prototype, and fail if the property does not exist in the prototype.

Comment 9

[Erik Tierney]

It is now an error to access any numeric property that is not an indexed property.  If the property is non numeric, then it is looked up in the prototype.

Comment 10

[Dan Schaffer]

verified the fix.  testcases are automated in test/acceptance/es4/nonindexproperty.as.