Closed Bug 409088 Opened 17 years ago Closed 16 years ago

Rewrite stack objects in garburator

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benjamin, Unassigned)

References

Details

This is an enhancement for garburator:

I will be manually marking certain classes as being stack allocated using the annotation __attribute__((stack))

If these classes have member nsCOMPtrs, they should be rewritten to be raw pointers just as if they occurred directly on the stack.

This will involve changing the initializer list in constructors:

class __attribute__((stack)) Example
{
public:
  Example(int i) : mI(i) { }

private:
  int mI;
  nsCOMPtr<nsIFoo> mFoo;
};

the constructor needs to become

Example(int i) : mI(i), mFoo(nsnull) { }

but of course only in cases where mFoo isn't already initialized.

I don't have anything checked in yet with this annotation, but I will soon.
Try it with latest oink/elsa revision. 

I implemented it in the testcase, I'll test it on Mozilla when you give me something to play with.
Status: NEW → ASSIGNED
I tried it... it seems that it's trying to add member initializers to destructor methods, not just constructors.

The real-world testcase is now up at actionmonkey+xpcomgc-patches, through the end (prerewrite_fixes.patch2).
Sorry, the real-world testcase is nsAutoPopupStatePusherExternal, and the error shows up as:

   }
 
   ~NS_AUTO_POPUP_STATE_PUSHER()
-  {
+: mWindow(nsnull)  {
     if (mWindow) {
       mWindow->PopPopupControlState(mOldState);
     }
   }
I think I failed to mention an important requirement ;-)

The members have to be rewritten, not just the constructors:

-  nsCOMPtr<nsIFoo> mFoo;
+  nsIFoo *mFoo;
Which implies that all the client code of those members will also need to be rewritten, as if those were stack nsCOMPtrs.
> Which implies that all the client code of those members will also need to be
> rewritten, as if those were stack nsCOMPtrs.
> 

Right, I think I have that detail covered now in the current pork. I don't have a tree setup to test this extensively, but I'll set one up if you continue having trouble. For now I just modified the testcase.
Appears to work now... I'll file new bugs if there are edge-cases that I find as I go.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
This work is making me tingle all over! I love you guys! *snif* (Ok, back to work! On to exceptions! :-P)

/be
Depends on: 430315
Product: Core → Firefox Build System
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.