Jens suggested that we could get precise error stacks if we had a wrapper around nsresult that would hold info about error propagation. I think I figured out how to do that.

Depends on D114241

Depends on D114242

Depends on D114243

Depends on D114244

Depends on D114245

Depends on D114338

Depends on D114339

Did you analyze the effect of QMResult on code generation?
I am not sure in how many cases we benefit from the unused-zero optimization of Result<V, nsresult> now.

I fear this will make it even more relevant to restrict the builds this gets into.

Maybe QMResult should better be called QMError if it's never used to indicate success.

(In reply to Simon Giesecke [:sg] [he/him] from comment #9)

Did you analyze the effect of QMResult on code generation?

Not yet, but maybe it's not that urgent if we add some ifdef, see below.

I fear this will make it even more relevant to restrict the builds this gets into.

That's actually a good observation, we can put all extra member variables in QMResult behind the ifdef that we already have for stuff like this.

Maybe QMResult should better be called QMError if it's never used to indicate success.

Originally I wanted to have the ability to use it without mozilla::Result, but maybe it's not needed and then we would rename it to QMError.

This makes it consistent with other places where we check EARLY_BETA_OR_EARLIER
and DEBUG identifier. For example HandleError.

Depends on D114340

The appening of module extra key was already commented out.

Ok, so I was able to limit the overhead to EARLY_BETA_OR_EARLIER || DEBUG builds.
sizeof(Result<Ok, nsresult) and sizeof(Result<Ok, QMResult) is the same in Release builds (the size is 4 bytes).

sizeof(Result<Ok, QMResult) in Nightly builds is 24 bytes.

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

Ok, so I was able to limit the overhead to EARLY_BETA_OR_EARLIER || DEBUG builds.
sizeof(Result<Ok, nsresult) and sizeof(Result<Ok, QMResult) is the same in Release builds (the size is 4 bytes).

sizeof(Result<Ok, QMResult) in Nightly builds is 24 bytes.

For the sake of readability I was just wondering if we should have some specifically labeled switch (#define) instead of sparse #if defined(EARLY_BETA_OR_EARLIER) || defined(DEBUG) throughout the code?

The new code for error stacks already has it, QM_ERROR_STACKS_ENABLED.

I was referring to the ones introduced in Bug 1709352 - Define LogError only when needed; r=#dom-storage, are those the same scope?

That can be changed too.

https://hg.mozilla.org/mozilla-central/rev/5ed477dfcd93
https://hg.mozilla.org/mozilla-central/rev/922a380b0d86
https://hg.mozilla.org/mozilla-central/rev/b4c891b1a31e
https://hg.mozilla.org/mozilla-central/rev/8313509e7cc1
https://hg.mozilla.org/mozilla-central/rev/e538b40f3e43
https://hg.mozilla.org/mozilla-central/rev/038434c75607
https://hg.mozilla.org/mozilla-central/rev/6d45855c91a7
https://hg.mozilla.org/mozilla-central/rev/f1905864fccd
https://hg.mozilla.org/mozilla-central/rev/bdfc8a9626c1
https://hg.mozilla.org/mozilla-central/rev/d239739c9637
https://hg.mozilla.org/mozilla-central/rev/fa9b64cfbfc9

LogError signature is now separately defined for the case when
QM_ERROR_STACKS_ENABLED is defined and when it's not defined). HandleError has
been changed as well to have explicit handling depending on
QM_ERROR_STACKS_ENABLED.

Depends on D116070

https://hg.mozilla.org/mozilla-central/rev/f65bf0f5a4d8

Depends on D116121

https://hg.mozilla.org/mozilla-central/rev/13f9d77af67b
https://hg.mozilla.org/mozilla-central/rev/0caad459ce02