617939 - Exact tracing of pointer arrays should use two-argument macros

Status: RESOLVED FIXED

(Tamarin Graveyard :: Tools, defect, P2)

Description

[Lars T Hansen]

In bug #604702 we read:

------ 8>< -------------------------------------------

> >   May be too late now, but would it be possible to change the
> >   exactgc.as script so that the GC_STRUCTURES, GC_POINTERS, GC_ATOMS
> >   (etc) declarations were written as e.g.:
> >     GC_POINTERS_SMALL(m_bases[1], m_baseCount);
> >   rather than:
> >     GC_POINTERS_SMALL(m_bases, 1, m_baseCount);
> >   I think it would reduce the cognitive overhead in reading the
> >   declarations.  (I would understand reluctance to have to jury-rig
> >   the parser to extract the field name from the array declaration, but
> >   it seems like even just hard-coding the special case of <id>[1]
> >   would be better than reading the presentation here.)
> 
> It is broken out the way it is because there are actually a few constant-
> length arrays in the code whose length is not 1, but I don't know if that
> actually matters any more.  I will check.

The script no longer needs to know that value, and it's not much work to fix
uses of the macro, but I think I understand why I did it this way.  The problem
is that arguments to the annotations can't contain arbitrary C++ expressions
except in certain contexts, in which case those expressions must be quoted (ie
represented as strings) - the script does not want to parse arbitrary C++
expressions, as evidenced by the already-hairy regexes.  That hack works well
enough because those quoted expressions are used only by the generator script
and not by the C++ code.  It's far from pretty.

In the common case where the expression is just a name or a constant the
parsing would be straightforward, however, so it's a question of whether the
cleaner syntax weighs up for restrictions on the field.  The restrictions could
then be lifted by having variant macros (GC_ATOMS3 comes to mind) that take
broken-out expressions.

------ 8>< -------------------------------------------

More than that, any expression not containing a comman would be straightforward to parse, presumably; we need to extract the identifier and separate it from an indexing expression but that should not be hard.

Comment 1

[Lars T Hansen]

Attachment: Patch

Changes the generator script and its documentation, the macros in GC.h, and uses of those macros throughout the code (not many).

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 501668 [details] [diff] [review]
Patch

R-: exactgc.as documentation (ie its comments) for GC_POINTERS still talks about three arguments.

But, a big "woo!" for the change itself.  :)  Looking forward to the next version.

Comment 3

[Lars T Hansen]

Attachment: Patch, v2

Now with updated documentation, and changed slightly to apply to TR tip (5750).

Comment 4

[Tamarin Bot]

changeset: 5753:d030d2c6f917
user:      Lars T Hansen <[e-mail]>
summary:   Fix 617939 - Exact tracing of pointer arrays should use two-argument macros (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/d030d2c6f917