Bug 154162 - GCC buildfix in Source/WebCore/svg/SVGToOTFFontConversion.cpp
Summary: GCC buildfix in Source/WebCore/svg/SVGToOTFFontConversion.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks: 154165
  Show dependency treegraph
 
Reported: 2016-02-12 04:20 PST by Csaba Osztrogonác
Modified: 2016-02-12 08:45 PST (History)
2 users (show)

See Also:


Attachments
Patch (1.83 KB, patch)
2016-02-12 04:22 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2016-02-12 04:20:16 PST
build error:
../../Source/WebCore/svg/SVGToOTFFontConversion.cpp: In member function 'size_t WebCore::SVGToOTFFontConverter::finishAppendingKERNSubtable(WTF::Vector<WebCore::SVGToOTFFontConverter::KerningData>, uint16_t)':
../../Source/WebCore/svg/SVGToOTFFontConversion.cpp:1100:30: error: use of 'kerningData' before deduction of 'auto'

Source/WebCore/svg/SVGToOTFFontConversion.cpp:
...
for (auto& kerningData : kerningData) {
     append16(kerningData.glyph1);
     append16(kerningData.glyph2);
     append16(kerningData.adjustment);
}
...

GCC can't handle properly if we use the same variable name in 
range declaration as in range expression of range-based for loop.
(tested with GCC 4.9 and 5.2 too.) 

The simplest workaround is renaming one of the variable.
Explicit type declaration isn't enough for GCC.

We needed the same workaround previously - bug151622 .
Comment 1 Csaba Osztrogonác 2016-02-12 04:22:57 PST
Created attachment 271164 [details]
Patch
Comment 2 Csaba Osztrogonác 2016-02-12 04:52:36 PST
I don't like at all to use the same variable name for the 
element and for the collection in range-based for loop,
but I think it is a GCC bug, so I reported it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69784
Comment 3 Andreas Kling 2016-02-12 05:23:37 PST
Comment on attachment 271164 [details]
Patch

huh!
Comment 4 Csaba Osztrogonác 2016-02-12 05:52:11 PST
Comment on attachment 271164 [details]
Patch

Clearing flags on attachment: 271164

Committed r196469: <http://trac.webkit.org/changeset/196469>
Comment 5 Csaba Osztrogonác 2016-02-12 05:52:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Darin Adler 2016-02-12 08:45:16 PST
Comment on attachment 271164 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271164&action=review

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1104
> +    for (auto& kerningDataElement : kerningData) {
> +        append16(kerningDataElement.glyph1);
> +        append16(kerningDataElement.glyph2);
> +        append16(kerningDataElement.adjustment);
>      }

I’m happy with this fix.

However, I wanted to point out for future reference in WebKit coding style that when a variable has really small scope like this, it’s often good practice to give it a shorter name since the context makes its meaning clear without the name having to express everything. Hence, I would have named this local variable "element" or "pair" rather than using a multi-word phrase.