723534 - Remove dead default cases.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: Remove dead default cases.

Clang warns on default cases that are never reached. This is currently breaking the build with --enable-warnings-as-errors. The attached patch fixes some cases.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 593846 [details] [diff] [review]
Remove dead default cases.

This patch is a bit riskier, let's get Ehsan to review it as his probably a more appropriate reviewer for stuff in content/.

Comment 2

[(no longer active)]

Comment on attachment 593846 [details] [diff] [review]
Remove dead default cases.

Review of attachment 593846 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not the right person to review the canvas changes.

::: content/smil/nsSMILTimedElement.cpp
@@ +2203,2 @@
>    }
> +  MOZ_NOT_REACHED("Switch is incomplete!");

Please don't change the debug string here.  Also shouldn't you return something here?

@@ +2275,2 @@
>    }
> +  MOZ_NOT_REACHED("Switch is incomplete!");

Ditto.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 593846 [details] [diff] [review]
Remove dead default cases.

The canvas changes seem fine to me.

Comment 4

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

(In reply to Ehsan Akhgari [:ehsan] from comment #2)
> Comment on attachment 593846 [details] [diff] [review]
> Remove dead default cases.
> 
> Review of attachment 593846 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not the right person to review the canvas changes.
> 
> ::: content/smil/nsSMILTimedElement.cpp
> @@ +2203,2 @@
> >    }
> > +  MOZ_NOT_REACHED("Switch is incomplete!");
> 
> Please don't change the debug string here.  Also shouldn't you return
> something here?

No, that is the point. The dynamics are:

* We build with -Werror
* If the switch is missing a case and has no default, the build fails.
* If the switch has all the cases and a default, the build succeeds

The net result is that without the default case, the build works if and only if all the cases ore covered, which is normally exactly what we need. The unreachable is there just to help other compilers and as documentation.

Comment 5

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: Remove dead default cases.

Comment 6

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=038c62997c55

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/038c62997c55