617079 - Existing API versioning selftests make assumptions about api-versions.xml

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

The existing API versions make assumptions about the ordinal values of api versions (660, 661, etc) in api-versions.xml, but more importantly, make assumptions about the relationship between them; this is suboptimal and means that the api versions can't be renumbered (as desired for some planned work) and some existing-but-unused versions can't be removed (FP_10_0_32 in particular).

We should rework the acceptance tests to avoid these assumptions, if possible.

Comment 1

[Steven Johnson]

Attachment: Patch

A bit of scope creep here, but generally good cleanup for subsequent changes in the pipeline:

-- ApiVersions become a well-defined enum; api-versions.h now is included in avmplus.h
-- the "-api" flag now requires a name rather than a value (eg "FP_9_0" rather than "660")
-- AvmCore now just references the well-known api version structure / constants directly, rather than requiring setAPIInfo() to be called; no client has ever specified anything other than the default value, nor does this seem likely in the future.
-- And finally, the api-versioning testcases in the shell, and the acceptance tests that exercise them, no longer depend on hardcoding actual integer versions being used in api versioning (they are looked up at runtime).

Note that this patch *does not* do anything to remove the implicit assumption about how the versions relate to each other; I decided that it's easier to keep this in place, as a fully general testcase is likely to be more trouble than it's worth, and the initial desire to remove some unused versions (in the name of saving bits in the API mask) is likely to be made moot by other changes that are underway.

Comment 2

[Jeff Dyer]

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

Nit, including ".cpp" might be out of style. I didn't apply the patch, but everything appears to be in order. Let me know if you want me to dig deeper.

Comment 3

[Steven Johnson]

including .cpp may or may not stay in subsequent patches, but FWIW, it's an existing required practice for (e.g.) builtin.cpp, so I'm not inclined to take heroic measures to avoid it unless others object.

Comment 4

[Steven Johnson]

pushed http://hg.mozilla.org/tamarin-redux/rev/44dbd74994f4