Closed Bug 168613 Opened 22 years ago Closed 13 years ago

Turn PtrBits into a class

Categories

(Core :: XPCOM, defect, P4)

x86
Windows XP
defect

Tracking

()

RESOLVED INCOMPLETE
mozilla1.3beta

People

(Reporter: john, Unassigned)

Details

(Whiteboard: [FIX])

Attachments

(1 file, 4 obsolete files)

The error-prone PtrBits abstraction, which reuses the empty bottom 2 bits of a pointer to an object (or a malloced pointer), is spreading through Mozilla like wildfire. This is good because it can save a significant amount of space through treating the PtrBits like a union with the lower bits telling us what the higher bits in the pointer mean. Since we are using it a lot, we should formalize it. This will also protect us against architectures in the future where the bottom 2 bits are *not* always zero--in such an architecture we can change this class to include a boolean and all will be well. We can make a fairly simple abstraction which captures the heart of this by storing a union with 2 or 4 void*'s and Int32s. Formalizing will also encourage use--I know I have sometimes decided to go the easy route of creating a PRInt32 flags or added a second pointer when I could have done a union and saved space. With such a class it would be easy enough to find an existing pointer, change it to PtrBits, and reuse unused bits. I will upload a file shortly with some classes to tinker with and think about.
Attached patch PtrUnion.h (obsolete) — Splinter Review
This has not been tested or compiled, but shows the shape of things to come. What do y'all think?
Status: NEW → ASSIGNED
Whiteboard: [FIX]
Attached patch PtrUnion.h (obsolete) — Splinter Review
That one had SetPtr() wrong. But the idea is the same, and the interface is the main thing right now :)
Attachment #99183 - Attachment is obsolete: true
scc, I'm curious where you see this going too. xpcom/base?
CC'ing others with a potential interest in the PtrBits abstraction.
OK, that one was sorta bogus. I'm going to start small with a PtrFlags class with an interface like this: class PtrFlags { public: void* get(); operator=(void* aFlags); PRBool GetFlag1(); PRBool GetFlag2(); void SetFlag1(PRBool aFlag); void SetFlag2(PRBool aFlag); }; And then graduate (hopefully) to: template<T> class PtrFlags { public: T* get(); T* operator->(); PtrFlags& operator=(T* aFlags); PRBool GetFlag1(); PRBool GetFlag2(); void SetFlag1(PRBool aFlag); void SetFlag2(PRBool aFlag); void Assign(T* aPtr, PRBool aFlag1, PRBool aFlag2); }; Union classes can use this as a jumping point. Thoughts?
The big drawback this has is that Flag1 and Flag2 are rather generic, making code less readable when used.
Yes, if a class wants to implement a helper function to get flag1 and flag2 with a better name it can and probably should; I didn't want to go and make a macro-based class when the user of the class can just as easily do it themselves. What this buys a user is freedom from brittle & and | statements and potentially allows them to work on other architectures if an architecture without the word alignment "feature" comes along.
How about making a macro which makes the flag bits more readable: NS_DECL_PTRBITS(mMyFlagPtrBits, Foo, Bar); which would expand to something like class mMyFlagPtrBitsClass : public nsPtrBits { PRBool getFoo() { return GetFlag1(); } PRBool getBar() { return GetFlag2(); } (with setFoo/etc) }; mMyFlagPtrBitsClass mMyFlagPtrBits; Though I'd like to suggest that instead of PRBool, you use PRPackedBool, because if you're already space consious, you might be assigning this into a PRPackedBool in another class, and you could avoid the inflation and deflation..
Attached patch PtrBits.h (working) (obsolete) — Splinter Review
This is by no means the final version, but this one has everything one would want in such an interface. Alec, the macro idea sounds good but I'd like to explore how it's going to be used--nsCheapSets.h, for example, needs to do more than just get/set, it is dynamically figuring out what to put in there. TODO: - templatize! - investigate macros - make it possible to also store / retrieve PRInt32 (a common use of PtrBits is to switch between storing a pointer and storing an int)
Attachment #99184 - Attachment is obsolete: true
Attached patch Templatized nsPtrBits.h (obsolete) — Splinter Review
This is a templatized version that does, for example, nsPtrBits<nsIForm>. It also supports Get/SetInt() and Get/SetInt2(), which allow you to store unsigned integers in the PtrBits as well (a thing we do in a couple of places at least). Finally, it has regression tests. I'll start converting some places over that use PtrBits tomorrow.
Attachment #99475 - Attachment is obsolete: true
Attached patch PatchSplinter Review
This makes a little templatized nsPtrBits and uses them in nsGenericElement.*. scc, could you look it over? I have some testing to do on my end--specifically compile on Mac and Linux--but I sure could use input on templates in general.
Attachment #99490 - Attachment is obsolete: true
Priority: -- → P4
Target Milestone: --- → mozilla1.3beta
Assignee: john → nobody
Status: ASSIGNED → NEW
QA Contact: scc → xpcom
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: