313
edits
Exceptions: Difference between revisions
no edit summary
(coding tasks) |
No edit summary |
||
| (6 intermediate revisions by the same user not shown) | |||
| Line 7: | Line 7: | ||
* Remove all current OOM handling code | * Remove all current OOM handling code | ||
* | * Create contracts for outparams | ||
* Rewrite call sites that ignore nsresults | * Rewrite call sites that ignore nsresults | ||
These steps are explained in more detail below. And by the way, we'd love to have community help with any of these. | * Rewrite call sites that use NS_SUCCEEDED | ||
These steps are explained in more detail below. And by the way, we'd love to have community help with any of these. Look for "Coding Task" below to find small but useful bites of work that you might be interested in starting on. | |||
= Improved OOM Handling = | = Improved OOM Handling = | ||
| Line 66: | Line 68: | ||
Coding Task 2: Dehydra GCC script to detect explicit tests for NS_OUT_OF_MEMORY return value. | Coding Task 2: Dehydra GCC script to detect explicit tests for NS_OUT_OF_MEMORY return value. | ||
= Contracts for Outparams = | |||
== Background == | |||
Some methods ("getters") 'return' a value other than an nsresult to the caller. Generally, the value is returned through a pointer-typed parameter, a.k.a., outparam. As of Mozilla 1.9, it's not always clear whether the returned value can be null if there is not error, or if the value can be changed if there is an error, etc., making things more confusing for callers. | |||
This situation needs to be cleared up for exceptions. With exceptions, we want these methods to return their value directly, which will have performance and developer sanity benefits. In that case, when there is an exception, no value is returned at all to the calling context. Thus, for all methods, '''when the error code is nonzero, outparams should not be modified'''. | |||
Some methods have the property that when the return code is NS_OK, the outparam is non-null (equivalently, when the outparam is non-null, the return code is nonzero). We want to identify these methods, because for them, a check for null is a valid way of checking for an error, and should be treated the same as an nsresult check for the other refactorings discussed here. | |||
== Code Changes == | |||
Coding Task 3: Create an analysis to check for methods that modify outparams when the return a nonzero nsresult. Eventually this analysis should be a gcc plugin so it can be integrated with the build process. | |||
Coding Task 4: Create an analysis to find methods that always return non-null outparams when the nsresult is NS_OK. Eventually, we want to annotate these methods with gcc attributes and enforce the annotation with a gcc plugin. | |||
= Fixing Ignored nsresults = | |||
== Background == | |||
There are a fair number of call sites that ignore nsresult return values. This can be for several reasons, including: | |||
* The caller checks failure using some other condition (e.g., a null return value) | |||
* The function being called always returns NS_OK. | |||
* At this call site, the caller has ensured that the function will succeed. | |||
* The caller doesn't need to respond to errors. | |||
These calls need checking before we can enable exceptions. In general, it won't be possible to ensure that a function doesn't throw an exception, especially if we use exceptions for OOM. Thus, call sites that now ignore nsresults need to be looked at and made exception safe. | |||
== Finding Ignored nsresults == | |||
The key need here is a tool to automatically find call sites that ignore return values. There is a script under development (by dmandelin) that does this, but it needs to be improved to handle all the special cases, such as checking for a null return value. | |||
Coding Task 5: Finish the ignored nsresult analysis. | |||
Once the list is in place, the calls will need manual attention. | |||
= Removing NS_SUCCEEDED = | = Removing NS_SUCCEEDED = | ||
| Line 97: | Line 139: | ||
This would be fairly easy to rewrite to the exceptions version, because the NS_FAILED check is easily identified as equivalent to letting the exception propagate to the caller. | This would be fairly easy to rewrite to the exceptions version, because the NS_FAILED check is easily identified as equivalent to letting the exception propagate to the caller. | ||
= Far Future Stuff = | |||
== | |||
This was the original plan for implementing exceptions, now categorized as far future. Also hopefully these steps will be easier once we've done the medium-term stuff documented above. | |||
== Benefits of Exceptions == | == Benefits of Exceptions == | ||