For Atom fields we need a HeapAtom class. The existing avmplus::AtomWB class serves this purposes, we should just eradicate the ATOM_WB macro and rename it HeapAtom (parity with Multiname approach). For Atom stack references I think we need an AtomRef class or somesuch which is a real class which has rules like GCRef (has to be used on stack not heap, can't be used to obtain pointers, can return ScriptObjectRef/StringRef etc).
Requirements: 1) Raw Atom type should be hidden from player/shell 2) It should be a compile error to put a HeapAtom class in anything other than a GCRoot/GCObject/GCFinalizedObject 3) It should be an assert (don't think compile time it possible) error to put an AtomRef in any other than the stack or a stack only allocated object Feedback requested: Are HeapAtom and AtomRef acceptable names? They would live in the avmplus namespace. Should AtomRef be an Atom in release builds and a class in debug builds (worried about performance, premature?)
Could we simply use GCMember<Atom> and GCRef<Atom>? I know there's a bit of overloading of conceptual machinery, but it seems that fewer names are better than more names. I'm sure that will run afoul of some existing nefarious uses of Atom in the Flash Player code base though... I'm not on board with HeapAtom - HeapHashtable is not a good name.
GCMember and GCRef assume T is a class, specifically one derived from GCObject or GCFinalizedObject. And the storage is set up as T* which isn't what we want. We have the Heap prefix in multiple places and its spreading, HeapMultiname, HeapList, HeapAtomList, HeapHashtable, HeapE4XNodeList, HeapNamespaceList. avmplus::AtomMember and avmplus::AtomRef?
I could absolutely live with AtomMember and AtomRef. They should presumably be in the same namespace as GCMember and GCRef, and all four should normally be available in the player without further qualification (IMO).
Is it impossible to use garden variety template specialization for GCMember<Atom> (etc) because of the type sniffing that we do?
(In reply to comment #3) > GCMember and GCRef assume T is a class, specifically one derived from GCObject > or GCFinalizedObject. And the storage is set up as T* which isn't what we > want. I took comment 2 to mean that we would introduce a distinct template specialization (and a *complete* specialization) for GCMember<Atom> and GCRef<Atom> that would not reuse the machinery in GCMember<T> or GCRef<T>, so it would not be setting up the storage in the same way. Assuming its feasible to set up such a specialization. (I'm not saying I like the conceptual overloading; it just seemed like there was miscommunication between comments 2 and 3. . .)
My bad, I can't think of any reason GCMember<Atom> and GCRef<Atom> wouldn't work with complete template specialization. My template foo is weak ;-)
I could live with either AtomMember+AtomRef, or GCMember<Atom>+GCRef<Atom>. Better yet, make both acceptable.
But for GCMember<Atom> to work, have to change Atom to be a unique type, not a uintptr. (Needs to be done anyway, it's just more work.)
(In reply to comment #9) > But for GCMember<Atom> to work, have to change Atom to be a unique type, not a > uintptr. (Needs to be done anyway, it's just more work.) Why? I thought we had established that we could specialize GCMember<Atom> to avoid any dependencies on it being a type. We want to change Atom to be a real class yes, but I don't think doing that needs to block the change to GCMember<Atom>.
templates can't specialize on a typedef; since "typedef intptr_t Atom", Atom looks like intptr_t to the type system, which means you could also do GCMember<int32_t>, which would be bad
(In reply to comment #11) > templates can't specialize on a typedef; since "typedef intptr_t Atom", Atom > looks like intptr_t to the type system, which means you could also do > GCMember<int32_t>, which would be bad I had assumed that such usage would, in the meantime, be caught during code reviews. But perhaps that is assuming too much. So maybe we should just go to AtomMember and AtomRef for now.
(In reply to comment #12) > I had assumed that such usage would, in the meantime, be caught during code > reviews. That would be nice; also, I would like a pony. (When it comes to glue code, assume that if it *can* be misused, it *will* be misused)
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 629746
bulk verifying resolved !fixed issues
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.