On Sat, Apr 23, 2016 at 1:29 AM, Nicholas Nethercote <[email protected] > wrote:
> I fully admit to not doing much JS engine work these days and I don't > know about this ReportOutOfMemory behaviour. But isn't the "return > null/false" pattern still heavily used? Looking quickly through > jsapi.cpp it sure seems to be. > Yes the "return null/false" pattern is still used, but when we OOM and call ReportOutOfMemory, we now also set a 'pending exception' on the JSContext. That sometimes complicates things, for instance when you want to ignore (or recover from) an OOM and forget to clear this pending exception, or when it's not clear who calls ReportOutOfMemory. > I would be happy to try to add more uses of it if people think it's > worthwhile. > IMO it's definitely worthwhile: I've added MOZ_WARN_UNUSED_RESULT to Vector, Jon added it to HashMap, and I've a patch in flight to add it to StringBuffer. Pretty much every time we do this, it catches new issues. > I also think it's offputtingly verbose and I have thought about a > shorter synonym so you can do something like |MOZ_WARN_UNUSED(bool)| > or even |MOZ_CHECK(bool)| instead of |MOZ_WARN_UNUSED_RESULT bool|. > Agreed. If we can make it less verbose, maybe we can add a new rule: "all fallible functions that return bool should have this annotation". That might also help the static analysis tool Jason talked about. Jan _______________________________________________ dev-tech-js-engine-internals mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
