829288 - fix a bunch of mingw warnings

Status: RESOLVED FIXED

(Core :: General, defect)

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: fix layout/ warnings on mingw

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: remove extra ';'s in content/

Comment 3

[Trevor Saunders (:tbsaunde)]

Attachment: fix a bunch of mingw warnings in xpcom/

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: some toolkit/ warnings

Comment 5

[Trevor Saunders (:tbsaunde)]

Attachment: fix warnings under browser/

Comment 6

[(no longer active)]

Comment on attachment 700677 [details] [diff] [review]
remove extra ';'s in content/

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

FWIW you don't really need to ask review for this kind of stuff!

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 700675 [details] [diff] [review]
fix layout/ warnings on mingw

># HG changeset patch
># User Trevor Saunders <[e-mail]>
>
>diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp
>index 10350a5..ea3853c 100644
>--- a/layout/base/nsPresContext.cpp
>+++ b/layout/base/nsPresContext.cpp
>@@ -2333,19 +2333,16 @@ enum InterruptMode {
>   ModeRandom,
>   ModeCounter,
>   ModeEvent
> };
> // Controlled by the GECKO_REFLOW_INTERRUPT_MODE env var; allowed values are
> // "random" (except on Windows) or "counter".  If neither is used, the mode is
> // ModeEvent.
> static InterruptMode sInterruptMode = ModeEvent;
>-// Used for the "random" mode.  Controlled by the GECKO_REFLOW_INTERRUPT_SEED
>-// env var.
>-static uint32_t sInterruptSeed = 1;

Looks like you're trying to remove an unused variable there -- but the variable is actually used.  Its only use is in an #ifndef XP_WIN block, though
 https://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2364
so you can just add "#ifndef XP_WIN" around the variable instead of dropping it.


>+++ b/layout/style/nsCSSRuleProcessor.cpp
>@@ -1051,18 +1051,18 @@ RuleCascadeData::AttributeListFor(nsIAtom* aAttribute)
> //
> 
> nsCSSRuleProcessor::nsCSSRuleProcessor(const sheet_array_type& aSheets,
>                                        uint8_t aSheetType,
>                                        Element* aScopeElement)
>   : mSheets(aSheets)
>   , mRuleCascades(nullptr)
>   , mLastPresContext(nullptr)
>-  , mSheetType(aSheetType)
>   , mScopeElement(aScopeElement)
>+  , mSheetType(aSheetType)
> {

I actually fixed this one on inbound this morning, in bug 828838. :) So this chunk isn't needed after all. (but thanks for catching it!)

>+++ b/layout/style/nsRuleNode.cpp
>@@ -3053,33 +3053,31 @@ nsRuleNode::SetFont(nsPresContext* aPresContext, nsStyleContext* aContext,
>       // XXXldb This platform-specific stuff should be in the
>       // LookAndFeel implementation, not here.
>       // XXXzw Should we even still *have* this code?  It looks to be making
>       // old, probably obsolete assumptions.
> 
>       // As far as I can tell the system default fonts and sizes
>       // on MS-Windows for Buttons, Listboxes/Comboxes and Text Fields are
>       // all pre-determined and cannot be changed by either the control panel
>-      // or programmtically.
>-      switch (fontID) {
>+      // or programatically.
>         // Fields (text fields)

nit: According to a google search, "programatically" wants two m's -- http://www.thefreedictionary.com/programmatically

Also: I think the the patch leaves this whole chunk indented 2 spaces too much -- could you fix that?

It might also make sense to place the "if" check _before_ the comment that starts with "// Fields (text fields)" -- so then that comment-block wouldn't have to change its indentation -- but it doesn't really matter, I'm happy either way.

>+++ b/layout/style/nsStyleSet.cpp
>@@ -290,17 +290,17 @@ SortStyleSheetsByScope(nsTArray<nsCSSStyleSheet*>& aSheets)
>   }
> }
> 
> nsresult
> nsStyleSet::GatherRuleProcessors(sheetType aType)
> {
>   mRuleProcessors[aType] = nullptr;
>   if (aType == eScopedDocSheet) {
>-    for (int i = 0; i < mScopedDocSheetRuleProcessors.Length(); i++) {
>+    for (uint32_t i = 0; i < mScopedDocSheetRuleProcessors.Length(); i++) {
[etc]
I actually fixed all these nsStyleSet warnings just this morning, too -- bug 829112. :)  So this chunk isn't needed.  Sorry for duplicated-effort there!

r=me with the above addressed.

Comment 8

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f9134833700b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b584bc588b40
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ab93edfbb1ea
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/eaab0e962e4d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/deab61ea49b6

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f9134833700b
https://hg.mozilla.org/mozilla-central/rev/b584bc588b40
https://hg.mozilla.org/mozilla-central/rev/ab93edfbb1ea
https://hg.mozilla.org/mozilla-central/rev/eaab0e962e4d
https://hg.mozilla.org/mozilla-central/rev/deab61ea49b6

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f9134833700b
https://hg.mozilla.org/mozilla-central/rev/b584bc588b40
https://hg.mozilla.org/mozilla-central/rev/ab93edfbb1ea
https://hg.mozilla.org/mozilla-central/rev/eaab0e962e4d
https://hg.mozilla.org/mozilla-central/rev/deab61ea49b6

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f9134833700b
https://hg.mozilla.org/mozilla-central/rev/b584bc588b40
https://hg.mozilla.org/mozilla-central/rev/ab93edfbb1ea
https://hg.mozilla.org/mozilla-central/rev/eaab0e962e4d
https://hg.mozilla.org/mozilla-central/rev/deab61ea49b6