ASSIGNED 165813
Replace ExceptionOr with std::expected
https://bugs.webkit.org/show_bug.cgi?id=165813
Summary Replace ExceptionOr with std::expected
JF Bastien
Reported 2016-12-13 12:10:14 PST
As mentioned in bug #164526 I'm not 100% sure it's a great idea, but we won't know without trying it out! I'm currently trying out std::expected in bug #163919, if that works out well I'll try out some of ErrorOr. I may need to implement more of the std::expected API (I was lazy and didn't do all of it yet). If the std::expected API isn't perfect we can also get the C++ proposal changed. I've already sent feedback to the author. The biggest thing I'd change right now is make std::expected a "delayed exception": stuff an error in it and if the user ignores it then it blows up (throws / aborts depending on exception support). Current proposal is here: https://github.com/viboes/std-make/blob/master/doc/proposal/expected/p0323r1.md Anyhow, this is a fun side-project.
Attachments
Darin Adler
Comment 1 2016-12-14 09:16:17 PST
I’ve spotted some places besides ExceptionOr that we could use std::expected. Examples: WebCore::STPProcessor::ParsingResult WebCore::SDPProcessor::generate WebCore::SDPProcessor::parse WebCore::SDPProcessor::generateCandidateLine WebCore::SDPProcessor::callScript Lots and lots of bool-returning functions named parse or parseXXX with out arguments, such as: WebCore::BaseDateAndTimeInputType::parseToDateComponents WebCore::parseHexColor WebCore::parseRepetitionType WebCore::ContentSecurityPolicyDirectiveList::parseDirective WebCore::parseBlendMode WebCore::parseCompositeAndBlendOperator WebCore::parseLineCap WebCore::parseLineJoin WebCore::parseTextAlign WebCore::parseTextBaseline WebCore::parseMetaHTTPEquivRefresh WebCore::parseMetaHTTPRefresh WebCore::parseDescriptors WebCore::parseQuad WebCore::parserSVGRect (the list goes on and on) Or maybe std::optional is what we want for most of these.
JF Bastien
Comment 2 2016-12-14 09:24:55 PST
(In reply to comment #1) > I’ve spotted some places besides ExceptionOr that we could use > std::expected. Examples: > > WebCore::STPProcessor::ParsingResult > > WebCore::SDPProcessor::generate > WebCore::SDPProcessor::parse > WebCore::SDPProcessor::generateCandidateLine > WebCore::SDPProcessor::callScript > > Lots and lots of bool-returning functions named parse or parseXXX with out > arguments, such as: > > WebCore::BaseDateAndTimeInputType::parseToDateComponents > WebCore::parseHexColor > WebCore::parseRepetitionType > WebCore::ContentSecurityPolicyDirectiveList::parseDirective > WebCore::parseBlendMode > WebCore::parseCompositeAndBlendOperator > WebCore::parseLineCap > WebCore::parseLineJoin > WebCore::parseTextAlign > WebCore::parseTextBaseline > WebCore::parseMetaHTTPEquivRefresh > WebCore::parseMetaHTTPRefresh > WebCore::parseDescriptors > WebCore::parseQuad > WebCore::parserSVGRect > (the list goes on and on) > > Or maybe std::optional is what we want for most of these. Thanks for the list! I'll go through these in sub-issues. On std::optional: I think we want to use it to denote "I'll give you a thing or not, but either way nothing went wrong". e.g. in WebAssembly.Memory's constructor the user can provide a "maximum" value or not, either way we're cool. I'd use std::expected when one of the outcomes (the unexpected one) is an error (that we're delaying into the std::expected object). I make this distinction because std::expected<void, E> makes a lot of sense to me: either we're all good but the code has nothing to give back, or the code has an error. For WebAssembly's validator I use this idiom: if validation succeeds then void is what you get, but if it fails you get an E. Do you think this high-level approach makes sense?
Darin Adler
Comment 3 2016-12-14 09:35:04 PST
(In reply to comment #2) > Do you think this high-level approach makes sense? Yes, *if* the client code ends up readable. My main concern is that the new idiom might be much harder to understand at call sites than the old one. The boolean thing was easy to understand: T result; if (!parse(result)) return BAD_FAILURE; There are lots of problems with that (initializing result just to ignore it when there is an error, requires if statement and so can't be used in expressions, etc.) but it's easy to understand. We have to make sure the most common ways of using the new idiom read really nicely. I feel like ExceptionOr was already treading in dangerous territory. The idiom I came up with to call a function and then propagate the exception to the caller was pretty wordy and not so elegant. I’m sure there are other styles possible including some really fancy stuff, but I want something easy to read, tidy and clean. As close as possible to the elegant invisibility of exceptions, without being invisible and thus a bit too mysterious.
Note You need to log in before you can comment on or make changes to this bug.