Will Rid Code of Outparams!

I resumed my outparam rewriting work last week. Having fixed the CPP induced architectural limitation that I ran into, it was quite straight-forward to factor out squash’s rewriting code into a new tool. Unlike squash, outparamdel (creatively named new tool), can rewrite code precisely and reliably. I still don’t have end-of-ast-node information in every Elsa AST member, but I think I have added position info to enough AST nodes to be able to do most of the Mozilla 2 rewrites.

Last time I was working on this, I was a little unhappy with the amount of code that had to be changed in the callee. I also wasn’t sure how to handle the complexity of callsites where control flow depends on the error code returned by the callee. This time I chose the path of least resistance.

In the callee two variables replace the out-parameter. The first holds the return value and the second points at the first and is declared in the same way as the out-parameter.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23

@@ -2358,6 +2358,5 @@
-nsresult
-nsCSSFrameConstructor::CreateHTMLImageFrame(nsIContent* aContent,
-                                            nsStyleContext* aStyleContext,
-                                            ImageFrameCreatorFunc aFunc,
-                                            nsIFrame** aFrame)
-{
+nsIFrame*
+nsCSSFrameConstructor::CreateHTMLImageFrame(nsIContent* aContent,
+                                            nsStyleContext* aStyleContext,
+                                            ImageFrameCreatorFunc aFunc)
+{
@@ -2364,1 +2364,3 @@
-  *aFrame = nsnull;
+  nsIFrame* __aFrame = 0;
+  nsIFrame** aFrame = &__aFrame;
+  *aFrame = nsnull;
@@ -2371,1 +2371,1 @@
-      return NS_ERROR_OUT_OF_MEMORY;
+      return nsnull;
@@ -2374,1 +2374,1 @@
-  return NS_OK;
+  return __aFrame;

With a little more work one can get of one or even both local variables, but that will require more heuristics.

I also simplified call-site rewriting by wrapping the call to the callee function into a ternary expression when the error code is evaluated.

@@ -5290,2 +5290,1 @@

1
2
3

-    rv = CreateHTMLImageFrame(aContent, aStyleContext, NS_NewImageFrame,
-                              &newFrame);
+    rv = ((newFrame = CreateHTMLImageFrame(aContent, aStyleContext, NS_NewImageFrame)) ? NS_OK : NS_ERROR_OUT_OF_MEMORY);

Obviously this could be improved upon by getting rid of rv altogether, but the trick works in all cases and can be improved-upon incrementally.

Problems & Limitations

• Code within macros is detected, but not modified. Since macros are something that is easier to modify manually than with a tool, this isn’t a problem.
• Indentation isn’t updated. I think this is solvable with a few more heuristics in the patcher.
• Expression rewriting doesn’t work on Mac or Windows source-code. Mac is going to be relatively straight-forward to support. MCPP doesn’t understand all of the Mac strangeness yet and needs a loving and affectionate mac user to teach it the ways of Darwin. Windows is more work as it will require both updating the mingw Mozilla port and porting MCPP. Coolness

1
2
3
4
5
6
7

nsCSSFrameConstructor.cpp:10383: nsCSSFrameConstructor::CreateContinuingTableFrame,6=newFrame,
nsCSSFrameConstructor.cpp:10993: nsCSSFrameConstructor::GetInsertionPoint,2=aParentFrame,
nsCSSFrameConstructor.cpp:2374: nsCSSFrameConstructor::CreateHTMLImageFrame,3=aFunc,
nsCSSFrameConstructor.cpp:4080: nsCSSFrameConstructor::ConstructDocElementTableFrame,2=childList,
nsCSSFrameConstructor.cpp:4477: nsCSSFrameConstructor::ConstructRootFrame,1=viewportFrame,
nsCSSFrameConstructor.cpp:4720: nsCSSFrameConstructor::ConstructRadioControlFrame,0=NS_NewGfxRadioControlFrame,
nsCSSFrameConstructor.cpp:4741: nsCSSFrameConstructor::ConstructCheckboxControlFrame,0=NS_NewGfxCheckboxControlFrame,

I wrote outparamdel and tested it on a convoluted manual testcase. Then I ran outparamdel mozilla-wide with the above input and the resulting code compiled! This was never the case prior to MCPP work since I found find new and exciting preprocessor madness to work-around and special case with every new rewrite.

Future

Going to make this a little more challenging by rewriting QueryInterface to return result by a return value which should affect the majority of functions in Mozilla.