756996 - GCC warnings in gfx/2d

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jacek Caban]

Attachment: fix v1.0

Unhandled enum value in switch statement is esp very noisy there. I've added defaults there, but probably in some cases default could be marked as unreachable. Other warnings were about static function was defained but not used (that's because they were in a headers and were not used by some header users; the fix requires marking them inline or moving to .cpp files). I've fixed also sign-compare warnings that seem safe in this case. The most useful seem to be unused variable warnings.

Comment 1

[Bas Schouten (:bas.schouten)]

Comment on attachment 625590 [details] [diff] [review]
fix v1.0

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

Looks fine mostly.

::: gfx/2d/DrawTargetD2D.cpp
@@ +87,5 @@
>  
>  ID2D1Factory *DrawTargetD2D::mFactory;
>  IDWriteFactory *DrawTargetD2D::mDWriteFactory;
>  
> +static bool IsPatternSupportedByD2D(const Pattern &aPattern)

Let's make these inline and -not- move them into cpp files. I don't want to move them again when other files need this in the future.

Comment 2

[Jacek Caban]

I've moved them back and marked inline:

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa3fe2aff1f5

Thanks for the review.

Comment 3

[Ed Morley [:emorley]]

Backed out for compilation failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=fe129d98513e

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bcf7ead7392

Comment 4

[Jacek Caban]

The failure was because MOZ_NOT_REACHED_MARKER is not present at all for GCC older than 4.5 in Assertion.h. I believe it's a bug (it should be at least defined as empty), but that's a separated issue. I've removed that part from my patch it builds on try:
https://tbpl.mozilla.org/?tree=Try&rev=1b14871c3acf

https://hg.mozilla.org/integration/mozilla-inbound/rev/50fa36a821fd

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

  MOZ_ASSERT(false);
  MOZ_NOT_REACHED_MARKER();

should just be

  MOZ_NOT_REACHED("Why");

really

Comment 6

[Jacek Caban]

That would use MOZ_Assert instead of its macro version MOZ_ASSERT, which AFAIR is not desired in this code.

Comment 7

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/mozilla-central/rev/50fa36a821fd