The default bug view has changed. See this FAQ.

GCC warnings in gfx/2d

RESOLVED FIXED in mozilla16

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

Trunk
mozilla16
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 625590 [details] [diff] [review]
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.
Attachment #625590 - Flags: review?(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.
Attachment #625590 - Flags: review?(bas.schouten) → review+
(Assignee)

Comment 2

5 years ago
I've moved them back and marked inline:

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

Thanks for the review.
Target Milestone: --- → mozilla15
Backed out for compilation failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=fe129d98513e

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bcf7ead7392
Target Milestone: mozilla15 → ---
(Assignee)

Comment 4

5 years ago
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
Target Milestone: --- → mozilla15
  MOZ_ASSERT(false);
  MOZ_NOT_REACHED_MARKER();

should just be

  MOZ_NOT_REACHED("Why");

really
(Assignee)

Comment 6

5 years ago
That would use MOZ_Assert instead of its macro version MOZ_ASSERT, which AFAIR is not desired in this code.
Target Milestone: mozilla15 → mozilla16
https://hg.mozilla.org/mozilla-central/rev/50fa36a821fd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.