1024322 - Replace new calls to MOZ_ASSUME_UNREACHABLE added in dom

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: remove-dom-MOZ_ASSUME_UNREACHABLE.patch

This patch is part of bug 990764, replacing calls to deprecated MOZ_ASSUME_UNREACHABLE with MOZ_CRASH or MOZ_ASSERT_UNREACHABLE (which is an assertion in all debug builds and release builds on the Nightly channel).

These calls to MOZ_ASSUME_UNREACHABLE in dom/ have crept in since I removed them all in a patch you reviewed in bug 990764 comment 24. :)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8438962 [details] [diff] [review]
remove-dom-MOZ_ASSUME_UNREACHABLE.patch

>+++ b/dom/datastore/DataStoreDB.cpp
>@@ -112,18 +112,17 @@ DataStoreDB::HandleEvent(nsIDOMEvent* aE
>   if (type.EqualsASCII("error") || type.EqualsASCII("blocked")) {
>     RemoveEventListeners();
>     mState = Inactive;
>     mCallback->Run(this, false);
>     mRequest = nullptr;
>     return NS_OK;
>   }
> 
>-  MOZ_ASSUME_UNREACHABLE("This should not happen");
>-  return NS_OK;
>+  MOZ_CRASH("This should not happen");
You need some return value here.
so keep return NS_OK;

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to Olli Pettay [:smaug] from comment #1)
> >-  MOZ_ASSUME_UNREACHABLE("This should not happen");
> >-  return NS_OK;
> >+  MOZ_CRASH("This should not happen");
> You need some return value here.
> so keep return NS_OK;

Why do we need a return value? The compiler (or at least the ones we use on the try servers) knows that the function won't return through that code path and doesn't emit a compilation error.

Comment 3

[Chris Peterson [:cpeterson]]

^ comment 2?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Oh, if it compiles, fine. That is just surprising.

Comment 5

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99682bd658ed

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/99682bd658ed