1275475 - Fix -Wlogical-op warning in dom/animation/ComputedTimingFunction.cpp

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: Wlogical-op_dom-animation.patch

dom/animation/ComputedTimingFunction.cpp:33:114 [-Wlogical-op] logical 'or' of collectively exhaustive tests is always true

This assertion in StepTiming() is always true because (aType != StepStart || aType != StepEnd) is true for any value of aType. :)

Comment 1

[Daisuke Akatsuka (:daisuke)]

Comment on attachment 8756194 [details] [diff] [review]
Wlogical-op_dom-animation.patch

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

Chris, thank you very much!

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c23a043fc70

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4c23a043fc70

Comment 4

[Brian Birtles (:birtles)]

Comment on attachment 8756194 [details] [diff] [review]
Wlogical-op_dom-animation.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1248532
[User impact if declined]: -Wlogical-op warning failure
[Describe test coverage new/current, TreeHerder]: Has baked on m-c for nearly a month
[Risks and why]: None. Just fixes an assertion from being useless to reporting what is was intended to.
[String/UUID change made/needed]: None

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8756194 [details] [diff] [review]
Wlogical-op_dom-animation.patch

Assertion fix, looks simple, ok to uplift to aurora.

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Brian, marking this fix-optional for beta as we are now heading into beta 7. It doesn't sound crucial to fix there but let me know if you disagree.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

This should have been nominated for beta approval rather than aurora approval at the time. Do you want to clear the aurora approval since it's meaningless in this context or just switch it over to beta instead?

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8756194 [details] [diff] [review]
Wlogical-op_dom-animation.patch

Thanks Ryan, I missed that one!