1672378 - Lift extensions to MOZ_TRY and related macros from dom/quota/QuotaCommon.h (QM_TRY_*)

Status: NEW

(Core :: MFBT, task, P3)

Description

[Simon Giesecke [:sg] [he/him]]

For the quota manager and its clients, several extensions to the mechanism provided by MOZ_TRY and MOZ_TRY_VAR are introduced, including:

• Split MOZ_TRY_VAR into MOZ_TRY_INSPECT and MOZ_TRY_UNWRAP, so that unwrapping can be avoided where not necessary.
• Allow variables to be declared by MOZ_TRY_INSPECT and MOZ_TRY_UNWRAP (which allows declaring const references/variables!)
• Report failures in a customizable way, e.g. via NS_DebugBreak/NS_WARNING.
• Allow the use in functions not returning an error, but having void return type or, e.g., where a nullptr should be returned to indicate an error.
• Allow customization of cleanup in case of a failure before returning.
• Allow asserting on failure.
• Add MOZ_TRY_RETURN to propagate both the result and a failure (including the customization options and reporting mentioned above).

These extensions could benefit the whole codebase by lifting them to mfbt/Result.h.

The original motivation for providing custom macros was the need to include extended error reporting. There is some challenge to make this general and customizable for the whole codebase, the default might be using NS_DebugBreak/NS_WARNING as mentioned above.

Parts of the macros in dom/quota/QuotaCommon.h might currently be specific to Result<T, nsresult>, but this limitation should be liftable.

Comment 1

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

When adding more extensions to control-flow-hiding macros, https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/xpcom/base/nsDebug.h#34-35 should be considered.

Comment 2

[Jan Varga [:janv]]

I was opposed to using "control-flow-hiding" macros for long time, but after introduction of mozilla::Result and MOZ_TRY I changed my mind.
There's a documentation for QM_TRY* macros, including a comparison of typical usage:
https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/dom/quota/QuotaCommon.h#96

Comment 3

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

(In reply to Jan Varga [:janv] from comment #2)

I was opposed to using "control-flow-hiding" macros for long time, but after introduction of mozilla::Result and MOZ_TRY I changed my mind.
There's a documentation for QM_TRY* macros, including a comparison of typical usage:
https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/dom/quota/QuotaCommon.h#96

If there's consensus about this among the ones owning the style guide (are there actually owners?), the recommendation to use MOZ_TRY etc. should presumably be added to https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/xpcom/base/nsDebug.h#34-35.

Comment 4

[Simon Giesecke [:sg] [he/him]]

It's worth to note that currently MOZ_TRY_VAR cannot be used at all with a non-default-constructible success type, but QM_TRY_UNWRAP/QM_TRY_INSPECT can. I came across this limitation when trying to use it with Result<std::reference_wrapper<T>, nsresult>.

Comment 6

[Jan Varga [:janv]]

Attachment: Bug 1672378 - Lift extensions to MOZ_TRY and related macros from dom/quota/QuotaCommon.h (QM_TRY_*); r=#dom-storage