1204400 - Fix -Wshadow warnings in gfx/thebes and suppress those from Skia headers

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: Wshadow_thebes.patch

Fix -Wshadow warnings in gfx/thebes and suppress those from Skia headers.

gfx/thebes/gfxCoreTextShaper.cpp:524:41 [-Wshadow] declaration shadows a local variable
gfx/thebes/gfxCoreTextShaper.cpp:534:45 [-Wshadow] declaration shadows a local variable
gfx/thebes/gfxMacPlatformFontList.mm:175:39 [-Wshadow] declaration shadows a local variable
gfx/thebes/gfxMacPlatformFontList.mm:868:21 [-Wshadow] declaration shadows a local variable
gfx/thebes/gfxMacPlatformFontList.mm:873:31 [-Wshadow] declaration shadows a local variable
gfx/thebes/gfxScriptItemizer.cpp:67:55 [-Wshadow] declaration shadows a field of 'gfxScriptItemizer'
gfx/thebes/gfxScriptItemizer.cpp:100:34 [-Wshadow] declaration shadows a field of 'gfxScriptItemizer'
gfx/thebes/gfxTextRun.cpp:1893:18 [-Wshadow] declaration shadows a local variable

GLContext.h:1540:60: error: declaration of 'ref' shadows a member of 'this' [-Werror=shadow]
GLContext.h:1546:90: error: declaration of 'ref' shadows a member of 'this' [-Werror=shadow]

gfx/thebes/gfxFontconfigFonts.cpp:1035:41: error: declaration of 'entry' shadows a previous local [-Werror=shadow]
gfx/thebes/gfxFT2FontList.cpp:937:19: error: declaration of 'faceList' shadows a previous local [-Werror=shadow-compatible-local]

../../../../workspace/gecko/gfx/skia/skia/include/core/SkPoint.h:30:28: error: declaration of 'y' shadows a member of 'this' [-Werror=shadow]
../../../../workspace/gecko/gfx/skia/skia/include/core/SkPoint.h:30:28: error: declaration of 'x' shadows a member of 'this' [-Werror=shadow]
...
../../../../workspace/gecko/gfx/skia/skia/include/core/SkTArray.h:92:41: error: declaration of 'count' shadows a member of 'this' [-Werror=shadow]
../../../../workspace/gecko/gfx/skia/skia/include/core/SkTArray.h:144:43: error: declaration of 'count' shadows a member of 'this' [-Werror=shadow]
../../../../workspace/gecko/gfx/skia/skia/include/core/SkTArray.h:373:75: error: declaration of 'count' shadows a member of 'this' [-Werror=shadow]
../../../../workspace/gecko/gfx/skia/skia/include/core/SkTArray.h:378:67: error: declaration of 'count' shadows a member of 'this' [-Werror=shadow]
../../../../workspace/gecko/gfx/skia/skia/include/gpu/GrRenderTarget.h:151:9: error: declaration of 'desc' shadows a member of 'this' [-Werror=shadow]
../../../../workspace/gecko/gfx/skia/skia/include/gpu/GrRenderTarget.h:151:9: error: declaration of 'isWrapped' shadows a member of 'this' [-Werror=shadow]

Comment 1

[Chris Peterson [:cpeterson]]

Comment on attachment 8660534 [details] [diff] [review]
Wshadow_thebes.patch

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

When one variable's name shadowed another's, I renamed both variables to ensure that I did not miss a renaming and leave a name referencing the wrong variable.

::: gfx/thebes/gfxCoreTextShaper.cpp
@@ -298,5 @@
>      double runWidth = ::CTRunGetTypographicBounds(aCTRun, ::CFRangeMake(0, 0),
>                                                    nullptr, nullptr, nullptr);
>  
>      nsAutoTArray<gfxShapedText::DetailedGlyph,1> detailedGlyphs;
> -    gfxShapedText::CompressedGlyph g;

This g was unused because the g variables below shadowed it.

::: gfx/thebes/gfxFcPlatformFontList.cpp
@@ +369,5 @@
>      }
>  
>      nsRefPtr<gfxCharacterMap> charmap;
>      nsresult rv;
> +    bool symbolFont = false; // currently ignored

Reuse this symbolFont variable and move "currently ignored" comment up here.

::: gfx/thebes/gfxMacPlatformFontList.mm
@@ +1107,5 @@
>                  uint32_t cmapLen = CFDataGetLength(cmapTable);
>                  nsRefPtr<gfxCharacterMap> charmap = new gfxCharacterMap();
>                  uint32_t offset;
> +                bool unicodeFont = false; // ignored
> +                bool symbolFont = false;

This symbolFont actually *is* used, so remove the "ignored" comment. Also move the definitions of unicodeFont and symbolFont down so all this block of definitions matches the function parameter order when they are passed to ReadCMAP() below.

Comment 2

[Benoit Girard (:BenWa)]

Comment on attachment 8660534 [details] [diff] [review]
Wshadow_thebes.patch

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

Thanks!

::: gfx/thebes/gfxMacPlatformFontList.mm
@@ +844,5 @@
>                                                     kCFAllocatorNull);
>          if (!str) {
>              return nullptr;
>          }
> +        index = 2;

len -> index is a non trivial change IMO, best keep these warning fix to have no impact. Why not use 'length' or something else similar if that's also used?

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Benoit Girard (:BenWa) from comment #2)
> len -> index is a non trivial change IMO, best keep these warning fix to
> have no impact. Why not use 'length' or something else similar if that's
> also used?

Thanks. I will use 'length' as you suggested.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d0b47f7a37b

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3d0b47f7a37b