154162 2016-02-12 04:20:16 -0800 GCC buildfix in Source/WebCore/svg/SVGToOTFFontConversion.cpp 2016-02-12 08:45:16 -0800 1 1 1 Unclassified WebKit New Bugs Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=151622 P2 Normal --- 154165 1 ossy ossy commit-queue ossy oldest_to_newest 1164101 0 ossy 2016-02-12 04:20:16 -0800 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 . 1164102 1 271164 ossy 2016-02-12 04:22:57 -0800 Created attachment 271164 Patch 1164109 2 ossy 2016-02-12 04:52:36 -0800 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 1164115 3 271164 kling 2016-02-12 05:23:37 -0800 Comment on attachment 271164 Patch huh! 1164124 4 271164 ossy 2016-02-12 05:52:11 -0800 Comment on attachment 271164 Patch Clearing flags on attachment: 271164 Committed r196469: <http://trac.webkit.org/changeset/196469> 1164125 5 ossy 2016-02-12 05:52:16 -0800 All reviewed patches have been landed. Closing bug. 1164152 6 271164 darin 2016-02-12 08:45:16 -0800 Comment on attachment 271164 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. 271164 2016-02-12 04:22:57 -0800 2016-02-12 05:52:11 -0800 Patch bug-154162-20160212132235.patch text/plain 1879 ossy U3VidmVyc2lvbiBSZXZpc2lvbjogMTk2NDY2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzEwODcxYjIwYTU4Mjc1 OWM5Yjk2MjQxMWMwZDAwMzNkY2Y5ZDExOC4uZDVlYzNlN2JjY2VmYTEwZGYwNTRhNzIwNGRmNzMx YzEzMDU5YzY5YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE2LTAyLTEyICBDc2Fi YSBPc3p0cm9nb27DoWMgIDxvc3N5QHdlYmtpdC5vcmc+CisKKyAgICAgICAgR0NDIGJ1aWxkZml4 IGluIFNvdXJjZS9XZWJDb3JlL3N2Zy9TVkdUb09URkZvbnRDb252ZXJzaW9uLmNwcAorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU0MTYyCisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBzdmcvU1ZHVG9PVEZG b250Q29udmVyc2lvbi5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTVkdUb09URkZvbnRDb252ZXJ0 ZXI6OmZpbmlzaEFwcGVuZGluZ0tFUk5TdWJ0YWJsZSk6CisKIDIwMTYtMDItMTEgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBbV2ViIElETF0gaW50ZXJmYWNlcyBz aG91bGQgaW5oZXJpdCBFdmVudFRhcmdldCBpbnN0ZWFkIG9mIGR1cGxpY2F0aW5nIHRoZSBFdmVu dFRhcmdldCBBUEkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3N2Zy9TVkdUb09URkZvbnRD b252ZXJzaW9uLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3N2Zy9TVkdUb09URkZvbnRDb252ZXJzaW9u LmNwcAppbmRleCA5NjRmN2IxNTM5MDZiNzI2Y2M2NDJjM2E4MmUyMzc0YzEzOWM5NDFkLi4wOGM5 OTg2ZjQxYTM4YzYwZGM1MTc5MWUzODM4ZDI0N2RmY2E5N2Q1IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9zdmcvU1ZHVG9PVEZGb250Q29udmVyc2lvbi5jcHAKKysrIGIvU291cmNlL1dlYkNv cmUvc3ZnL1NWR1RvT1RGRm9udENvbnZlcnNpb24uY3BwCkBAIC0xMDk3LDEwICsxMDk3LDEwIEBA IHNpemVfdCBTVkdUb09URkZvbnRDb252ZXJ0ZXI6OmZpbmlzaEFwcGVuZGluZ0tFUk5TdWJ0YWJs ZShWZWN0b3I8S2VybmluZ0RhdGE+IGtlCiAgICAgYXBwZW5kMTYoKGtlcm5pbmdEYXRhLnNpemUo KSAtIHJvdW5kZWROdW1LZXJuaW5nUGFpcnMpICogNik7IC8vIHJhbmdlU2hpZnQ6ICJUaGUgdmFs dWUgb2YgblBhaXJzIG1pbnVzIHRoZSBsYXJnZXN0IHBvd2VyIG9mIHR3byBsZXNzIHRoYW4gb3Ig ZXF1YWwgdG8gblBhaXJzLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgLy8gYW5kIHRoZW4gbXVsdGlwbGllZCBi eSB0aGUgc2l6ZSBpbiBieXRlcyBvZiBhbiBlbnRyeSBpbiB0aGUgdGFibGUuIgogCi0gICAgZm9y IChhdXRvJiBrZXJuaW5nRGF0YSA6IGtlcm5pbmdEYXRhKSB7Ci0gICAgICAgIGFwcGVuZDE2KGtl cm5pbmdEYXRhLmdseXBoMSk7Ci0gICAgICAgIGFwcGVuZDE2KGtlcm5pbmdEYXRhLmdseXBoMik7 Ci0gICAgICAgIGFwcGVuZDE2KGtlcm5pbmdEYXRhLmFkanVzdG1lbnQpOworICAgIGZvciAoYXV0 byYga2VybmluZ0RhdGFFbGVtZW50IDoga2VybmluZ0RhdGEpIHsKKyAgICAgICAgYXBwZW5kMTYo a2VybmluZ0RhdGFFbGVtZW50LmdseXBoMSk7CisgICAgICAgIGFwcGVuZDE2KGtlcm5pbmdEYXRh RWxlbWVudC5nbHlwaDIpOworICAgICAgICBhcHBlbmQxNihrZXJuaW5nRGF0YUVsZW1lbnQuYWRq dXN0bWVudCk7CiAgICAgfQogCiAgICAgcmV0dXJuIHNpemVPZktlcm5pbmdEYXRhVGFibGU7Cg==