484509 - abcasm: need support for a[i] style setproperty

Status: RESOLVED FIXED

(Tamarin Graveyard :: Tools, defect)

Description

[Tom Harwood]

i.e., setproperty null

Comment 1

[Lars T Hansen]

Attachment: General multiname support

A simple change that adds general support for multinames using a straightforward syntax (see comment in the grammar, included in this patch).

The patch does not incorporate the regenerated jar file; ping me if you want it.

Comment 2

[Tom Harwood]

In general I like the way this is going.  A couple of comments:

- The use of the @ character does not conform to its use in AS3, where it specifies an attribute variant of a name.

- The proto-AET backing abcasm is starting to co-evolve with the actual AET.  I don't think that's a reason to avoid necessary maintenance to abcasm, but at some point these subsystems need to be made congruent.  That could be done by making abcasm a driver and grammar that drives the AET, as long as we don't introduce incompatible semantics at the AET level.

Comment 3

[Lars T Hansen]

Attachment: General multiname support, v2

Adds support for CONSTANT_TypeName and QNames in multiname instructions.  Updated comment block.  

Note, I ran tabify on these files because they predominantly use tabs despite the modeline that says not to use tabs, so there are a few strange changes in this diff.  That appears to be a win over the otherwise mixed indentation that otherwise resulted.

Comment 4

[Tom Harwood]

Comment on attachment 554040 [details] [diff] [review]
General multiname support, v2

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

Looks OK, but why use # on typenames?  ANTLR should be able to handle <typename> style constructs, which would look a lot more like AS3 and might some day get us closer to a generalized typename.

Comment 5

[Lars T Hansen]

Agreed, the # was a bit hacky, more for expedience than anything.  I'll consider testing ".<" ... ">" to see how that works.

Comment 6

[Edwin Smith]

Comment on attachment 554040 [details] [diff] [review]
General multiname support, v2

seems fine to me if it works and you guys are happy with it.  limiting scope creep probably trumps any of my notes.

nits, as you see fit:
- maybe fix the modelines to agree with the tabified files?
- xml attribute names are naturally written as @foobar... as long as IDENTIFIER sucks in the @ character, no conflict, right?

fyi, in verbose dumps I've habitually printed [] in places where an index or namespace gets substituted into a multiname.  not sure how well that would parse.

Comment 7

[Lars T Hansen]

I've adopted both the V.<C> and [] syntaxen without any problems and I'll just land the patches with those changes without re-review.

Comment 8

[Tamarin Bot]

changeset: 6530:6b300f7377e5
user:      Lars T Hansen <[e-mail]>
summary:   Fix 484509: - abcasm: need support for a[i] style setproperty (r=edwsmith, f=tharwood)

http://hg.mozilla.org/tamarin-redux/rev/6b300f7377e5