560412 - Publicly expose the value of LazilyConstructed<T> objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ben Newman (:bnewman) (:benjamn)]

Currently LazilyConstructed<T> values can be constructed, but the constructed values can't be accessed outside the class.  I propose adding

  T *address();
  const T *address() const;

Also, it seems that the const empty() function has trouble calling the non-const constructed() method, which has presumably been ok until now because empty() was never called.  I can fix that too while I'm at it.

Comment 1

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #0)
>   const T *address() const;

This isn't really necessary, on second thought, because LazilyConstructed<T> objects will never be immutable.

Comment 2

[Luke Wagner [:luke]]

Sounds good.  Incidentally, TM tip has a newer version of LazilyConstructed (silencing bloody GCC strict-aliasing warnings) which you probably want to work against.

Also, is there any chance we could get a return-by-reference member instead of or in addition to address()?  Also, the ever-popular AutoValueRooter calls their return-by-pointer addr(), so perhaps that abbreviation could be used for consistency?

Comment 3

[Ben Newman (:bnewman) (:benjamn)]

Attachment: Patch adding the T* address(); method.

I was torn between implementing empty() as

  return !static_cast<LazilyConstructed*>(this)->constructed();

or simply

  return !bytes[sizeof(T)];

I went with the latter for brevity, although it duplicates the converted() code.

Comment 4

[Ben Newman (:bnewman) (:benjamn)]

These comments passed (without collision??) in mid-air.  I like your suggestions and will update momentarily.

Comment 5

[Ben Newman (:bnewman) (:benjamn)]

Attachment: Patch shortening the address() method to addr() and introducing a ref() method.

This applies to mozilla-central.  I'll post a TM patch next.

Comment 6

[Ben Newman (:bnewman) (:benjamn)]

Attachment: TM-relative patch.

As promised.

Comment 7

[Luke Wagner [:luke]]

Comment on attachment 440106 [details] [diff] [review]
TM-relative patch.

Thank you, sir

Comment 8

[Ben Newman (:bnewman) (:benjamn)]

Does this need any more review before landing (either on TM or mozilla-central, or both)?

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Nope, Luke's review is sufficient.

Comment 10

[Ben Newman (:bnewman) (:benjamn)]

Pushed to tracemonkey:
http://hg.mozilla.org/tracemonkey/rev/b22a8aee5311

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/b22a8aee5311