Remove dead default cases.

RESOLVED FIXED in mozilla13

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

unspecified
mozilla13
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 593846 [details] [diff] [review]
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.
Attachment #593846 - Flags: review?(jmuizelaar)
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/.
Attachment #593846 - Flags: review?(jmuizelaar) → review?(ehsan)
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.
Attachment #593846 - Flags: review?(ehsan)
Comment on attachment 593846 [details] [diff] [review]
Remove dead default cases.

The canvas changes seem fine to me.
(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.
Created attachment 594091 [details] [diff] [review]
Remove dead default cases.
Assignee: nobody → respindola
Attachment #593846 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #594091 - Flags: review?(ehsan)
Attachment #594091 - Flags: review?(ehsan) → review+
Depends on: 723114
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=038c62997c55
https://hg.mozilla.org/mozilla-central/rev/038c62997c55
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.