Exceptions: Difference between revisions
(first version, just a copy of an email with a new header.) |
(wikified and revised sec1) |
||
Line 1: | Line 1: | ||
This is a discussion document for ideas about how to use static analysis to help refactor Mozilla to use C++ exceptions instead of nsresults. | This is a discussion document for ideas about how to use static analysis to help refactor Mozilla to use C++ exceptions instead of nsresults. In particular, this is about how to modify the call sites of methods so that the code uses exception handling and is exception safe. | ||
=== Identifying nothrow methods === | |||
Methods that never fail are the easiest to handle: exception safety at call sites is free. These are called '''nothrow''' methods following the ''Effective C++'' terminology. If we identify nothrow methods and annotate them, then we can easily refactor their sites. | |||
== nothrow analysis == | |||
There are three ways a method can fail: | |||
* By returning a non-zero (non-NS_OK) value, | |||
* By calling another nsresult method that can fail (i.e., is not nothrow), | |||
* By using a C++ feature that can throw an exception. The primary example is allocating memory with '''new'''. | |||
The static analysis must be designed to detect any of these conditions. Because it's hard to know what method will be called in C++, a good starting point for the analysis would be to say that a method is nothrow if it can only return 0, does not invoke new, and does not call any other method. If this doesn't pick up enough nothrow methods, we can either do it by hand or augment the analysis with a simple call graph construction. | |||
== Rewriting nothrow call sites == | |||
Some nothrow call sites may already ignore the return value. These call sites can be left alone (for exception rewriting--but [[outparamdel]] will rewrite many of them). | |||
Otherwise, the call site should look like this: | |||
nsresult rv = callMethod(); | |||
if (NS_FAILED(rv)) { | |||
stuff; | |||
} | |||
Logically, this should be rewritten to: | |||
callMethod(); | |||
nsresult rv = 0; | |||
if (NS_FAILED(rv)) { | |||
stuff; | |||
} | |||
At this point, a standard branch folding and dead code elimination should be able to clean up the code to the desired: | |||
callMethod(); | |||
2. ignored nsresults | 2. ignored nsresults |
Revision as of 22:41, 20 December 2007
This is a discussion document for ideas about how to use static analysis to help refactor Mozilla to use C++ exceptions instead of nsresults. In particular, this is about how to modify the call sites of methods so that the code uses exception handling and is exception safe.
Identifying nothrow methods
Methods that never fail are the easiest to handle: exception safety at call sites is free. These are called nothrow methods following the Effective C++ terminology. If we identify nothrow methods and annotate them, then we can easily refactor their sites.
nothrow analysis
There are three ways a method can fail:
- By returning a non-zero (non-NS_OK) value,
- By calling another nsresult method that can fail (i.e., is not nothrow),
- By using a C++ feature that can throw an exception. The primary example is allocating memory with new.
The static analysis must be designed to detect any of these conditions. Because it's hard to know what method will be called in C++, a good starting point for the analysis would be to say that a method is nothrow if it can only return 0, does not invoke new, and does not call any other method. If this doesn't pick up enough nothrow methods, we can either do it by hand or augment the analysis with a simple call graph construction.
Rewriting nothrow call sites
Some nothrow call sites may already ignore the return value. These call sites can be left alone (for exception rewriting--but outparamdel will rewrite many of them).
Otherwise, the call site should look like this:
nsresult rv = callMethod(); if (NS_FAILED(rv)) { stuff; }
Logically, this should be rewritten to:
callMethod(); nsresult rv = 0; if (NS_FAILED(rv)) { stuff; }
At this point, a standard branch folding and dead code elimination should be able to clean up the code to the desired:
callMethod();
2. ignored nsresults
Once the nothrows are identified and put aside, I'd like to assume that all other call sites that use nsresult methods "do the right thing". But there may be call sites that ignore the return value, which would make them potentially exception unsafe. So the second step is an analysis to flag all of these locations.
Roughly, the analysis should flag a call site where:
(a) the method is not nothrow (b) there is no branch on NS_FAILED(rv) or NS_SUCCEEDED(rv)
The easy way to do (b) is to just look for if statements where the condition refers to NS_FAILED or NS_SUCCEEDED on the variable where the result goes. But that's not perfect, because of things like:
- Calling NS_FAILED directly on the function without a variable - Assigning the rv or the NS_FAILED test result to a variable then branching later - Overwriting the value of rv before the test (so the rv isn't actually branched on)
The easy way might be good enough for our purposes. If not, we can design an abstract interpretation that is more precise.
3. tested nsresults
This is all the nsresult call sites not already covered. Finally, the hard one. My first thought is that we can look at the success and failure paths through the code and rearrange them to use exceptions. In most cases, the test should just be an if that branches on NS_FAILED or NS_SUCCEEDED, so that we can easily identify the success path and the failure path. Then, we can (logically) rewrite the code so it has this form:
nsresult rv = callSomeMethod(); maybe some_junk(); if (NS_FAILED(rv)) { maybe fixup(); maybe return rv; maybe return NS_ERROR_FOO; maybe return 0; } else { maybe do_stuff(); }
Having some_junk() would be very annoying--hopefully it is rare and we can just flag it for manual fixing. Otherwise, we rewrite the code to use exceptions. I think there are several distinct rewrite patterns depending on the return values and the presence of the various fixups. Hopefully they can be fit into a small number of basic categories.
- Case 1: return rv in orig code
FixupMgr f; callSomeMethod(); maybe do_stuff();
- Case 2: all others
try { callSomeMethod(); maybe do_stuff(); } except (nserror e) { maybe fixup(); maybe throw NS_ERROR_FOO; maybe return; }
In case 2 it is also an option to use a FixupMgr.
One significant glitch is see is when the error handling block uses constructs like
X1. NS_FAILED(rv) || foo X2. NS_FAILED(rv) && foo X3. NS_FAILED(rv1) || NS_FAILED(rv2) X4. NS_FAILED(rv1) && NS_SUCCEEDED(rv2)
These seem to be pretty common. Some thoughts on handling them:
X1. Here we can duplicate the failure code. The code to be dup'd should be small if we use FixupMgrs.
X2. Here we can guard the catch block actions with the additional condition foo.
X3 & X4 seem harder, maybe not worth thinking about right now. Hopefully there are not too many. X1 and X2 are common, though, I think.
Dave