313
edits
Exceptions: Difference between revisions
rewrote & wikified s2
(wikified and revised sec1) |
(rewrote & wikified s2) |
||
| 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. In particular, this is about how to modify the call sites of methods so that the code uses exception handling and is exception safe. | 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. | ||
=== | === 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. | 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. | ||
| Line 40: | Line 40: | ||
callMethod(); | callMethod(); | ||
=== Call sites that ignore return values === | |||
Once | Once nothrows are identified and taken care of, all the remaining call sites need to be made exception safe. Static analysis has no way of knowing the safety requirements, so we can't create a static analysis that just makes everything exception safe. But it seems reasonable to assume that existing call sites that test the nsresult do the right thing in the presence of errors, and then we can make a rewriting tool that will ensure the same thing is done with exceptions. | ||
But first we need to take care of existing call sites that just ignore errors, because (a) that looks like a bug, and (b) if they are supposed to ignore errors, they'll need to be rewritten to ignore exceptions. | |||
== Analysis to detect ignored return values == | |||
Roughly, the analysis should flag all call sites where: | |||
* the method is not nothrow | |||
* there is no branch on NS_FAILED(rv) or NS_SUCCEEDED(rv) | |||
The easy way is to look for '''if''' statements where the condition contains NS_FAILED(rv) or NS_SUCCEEDED(rv). 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. | The easy way might be good enough for our purposes. If not, we can design an abstract interpretation that is more precise. | ||
== Rewriting ignored return values == | |||
If it turns out most of these are bugs, we can just flag them for manual repair. | |||
On the other hand, if almost all of these sites are correct code, then we can handle an ignore site like this: | |||
callMethod(); | |||
by rewriting it to this: | |||
nsresult rv = callMethod(); | |||
if (NS_FAILED(rv)) {} | |||
and letting the full rewriting algorithm rewrite this to use exceptions. Or, we could rewrite it directly as: | |||
try { | |||
callMethod(); | |||
} except (nserror e) {} | |||
3. tested nsresults | 3. tested nsresults | ||