197148 2019-04-21 08:08:56 -0700 [aarch64] clang Wc++11-narrowing for unsigned char in WebCore/contentextensions 2019-04-24 08:52:19 -0700 1 1 1 Unclassified WebKit Platform WebKit Local Build Unspecified Other RESOLVED FIXED InRadar P2 Normal --- 1 wk webkit-unassigned achristensen ap commit-queue mcatanzaro webkit-bug-importer oldest_to_newest 1529143 0 367910 wk 2019-04-21 08:08:56 -0700 Created attachment 367910 webkit-schar.patch Building WebKitGtk+ 2.24.0 on FreeBSD/aarch64 (13-CURRENT) results in signed char warnings (that get turned into errors by Werror): http://thunderx1.nyi.freebsd.org/data/head-arm64-default/p498440_s346039/logs/errors/webkit2-gtk3-2.24.0_1.log /wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.24.0/Source/WebCore/contentextensions/DFACombiner.cpp:109:52: error: non-constant-expression cannot be narrowed from type 'char' to 'signed char' in initializer list [-Wc++11-narrowing] m_output.transitionRanges.append({ range.first, range.last }); ^~~~~~~~~~~ /wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.24.0/Source/WebCore/contentextensions/DFACombiner.cpp:109:52: note: insert an explicit cast to silence this issue m_output.transitionRanges.append({ range.first, range.last }); ^~~~~~~~~~~ static_cast<signed char>( ) /wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.24.0/Source/WebCore/contentextensions/DFACombiner.cpp:109:65: error: non-constant-expression cannot be narrowed from type 'char' to 'signed char' in initializer list [-Wc++11-narrowing] m_output.transitionRanges.append({ range.first, range.last }); ^~~~~~~~~~ /wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.24.0/Source/WebCore/contentextensions/DFACombiner.cpp:109:65: note: insert an explicit cast to silence this issue m_output.transitionRanges.append({ range.first, range.last }); ^~~~~~~~~~ static_cast<signed char>( ) The attached patch fixes this by using signed chars in the types of the 'range' objects. 1529465 1 367910 ap 2019-04-22 16:46:53 -0700 Comment on attachment 367910 webkit-schar.patch View in context: https://bugs.webkit.org/attachment.cgi?id=367910&action=review > Source/WebCore/contentextensions/DFACombiner.cpp:40 > + typedef MutableRangeList<signed char, uint64_t, 128> CombinedTransitionsMutableRangeList; WebKit coding style is to use one word (signed/unsigned, not signed char/unsigned char). I don't know if we necessarily want to fix this warning in code, it looks overly pedantic. But to have this discussed, it will be useful to formally post it for review (with a ChangeLog, and marked r?). Would you be willing to do that? 1529468 2 367910 achristensen 2019-04-22 16:52:07 -0700 Comment on attachment 367910 webkit-schar.patch View in context: https://bugs.webkit.org/attachment.cgi?id=367910&action=review >> Source/WebCore/contentextensions/DFACombiner.cpp:40 >> + typedef MutableRangeList<signed char, uint64_t, 128> CombinedTransitionsMutableRangeList; > > WebKit coding style is to use one word (signed/unsigned, not signed char/unsigned char). > > I don't know if we necessarily want to fix this warning in code, it looks overly pedantic. But to have this discussed, it will be useful to formally post it for review (with a ChangeLog, and marked r?). Would you be willing to do that? This is true with things like integers, but chars are a mess. They're usually unsigned on linux and signed most other places but not all. This needs the signed keyword to be portable. If you request review and commit, I'll do both. 1529469 3 achristensen 2019-04-22 16:53:03 -0700 You'll also need a ChangeLog entry. Running Tools/Scripts/prepare-ChangeLog will give you a template to fill in. 1529582 4 mcatanzaro 2019-04-23 07:54:06 -0700 Yeah, I agree this change is correct. signed or unsigned on its own would be signed int or unsigned int, not what's needed here. Remember signed char is a separate type from char (unlike all other signed types). 1529604 5 368036 wk 2019-04-23 08:51:57 -0700 Created attachment 368036 webkit-schar.patch v2 added changelog entry 1529979 6 368036 achristensen 2019-04-24 08:24:53 -0700 Comment on attachment 368036 webkit-schar.patch v2 See https://github.com/llvm/llvm-project/blob/master/lldb/source/Utility/ArchSpec.cpp#L763 for why this kind of thing is necessary :/ 1529992 7 368036 commit-queue 2019-04-24 08:51:45 -0700 Comment on attachment 368036 webkit-schar.patch v2 Clearing flags on attachment: 368036 Committed r244591: <https://trac.webkit.org/changeset/244591> 1529993 8 commit-queue 2019-04-24 08:51:47 -0700 All reviewed patches have been landed. Closing bug. 1529994 9 webkit-bug-importer 2019-04-24 08:52:19 -0700 <rdar://problem/50169088> 367910 2019-04-21 08:08:56 -0700 2019-04-23 08:51:57 -0700 webkit-schar.patch webkit-schar.patch text/plain 1259 wk LS0tIFNvdXJjZS9XZWJDb3JlL2NvbnRlbnRleHRlbnNpb25zL0RGQUNvbWJpbmVyLmNwcC5vcmln CTIwMTktMDQtMjEgMTQ6Mzk6MzAgVVRDCisrKyBTb3VyY2UvV2ViQ29yZS9jb250ZW50ZXh0ZW5z aW9ucy9ERkFDb21iaW5lci5jcHAKQEAgLTM3LDcgKzM3LDcgQEAgbmFtZXNwYWNlIFdlYkNvcmUg ewogbmFtZXNwYWNlIENvbnRlbnRFeHRlbnNpb25zIHsKIAogY2xhc3MgREZBTWVyZ2VyIHsKLSAg ICB0eXBlZGVmIE11dGFibGVSYW5nZUxpc3Q8Y2hhciwgdWludDY0X3QsIDEyOD4gQ29tYmluZWRU cmFuc2l0aW9uc011dGFibGVSYW5nZUxpc3Q7CisgICAgdHlwZWRlZiBNdXRhYmxlUmFuZ2VMaXN0 PHNpZ25lZCBjaGFyLCB1aW50NjRfdCwgMTI4PiBDb21iaW5lZFRyYW5zaXRpb25zTXV0YWJsZVJh bmdlTGlzdDsKIAogICAgIGVudW0gY2xhc3MgV2hpY2hERkEgewogICAgICAgICBBLAotLS0gU291 cmNlL1dlYkNvcmUvY29udGVudGV4dGVuc2lvbnMvTkZBVG9ERkEuY3BwLm9yaWcJMjAxOS0wNC0y MSAxNDo0MDo1MyBVVEMKKysrIFNvdXJjZS9XZWJDb3JlL2NvbnRlbnRleHRlbnNpb25zL05GQVRv REZBLmNwcApAQCAtNDEsOSArNDEsOSBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKIG5hbWVzcGFj ZSBDb250ZW50RXh0ZW5zaW9ucyB7CiAKLXR5cGVkZWYgTXV0YWJsZVJhbmdlPGNoYXIsIE5GQU5v ZGVJbmRleFNldD4gTkZBTm9kZVJhbmdlOwotdHlwZWRlZiBNdXRhYmxlUmFuZ2VMaXN0PGNoYXIs IE5GQU5vZGVJbmRleFNldD4gTkZBTm9kZVJhbmdlTGlzdDsKLXR5cGVkZWYgTXV0YWJsZVJhbmdl TGlzdDxjaGFyLCBORkFOb2RlSW5kZXhTZXQsIDEyOD4gUHJlYWxsb2NhdGVkTkZBTm9kZVJhbmdl TGlzdDsKK3R5cGVkZWYgTXV0YWJsZVJhbmdlPHNpZ25lZCBjaGFyLCBORkFOb2RlSW5kZXhTZXQ+ IE5GQU5vZGVSYW5nZTsKK3R5cGVkZWYgTXV0YWJsZVJhbmdlTGlzdDxzaWduZWQgY2hhciwgTkZB Tm9kZUluZGV4U2V0PiBORkFOb2RlUmFuZ2VMaXN0OwordHlwZWRlZiBNdXRhYmxlUmFuZ2VMaXN0 PHNpZ25lZCBjaGFyLCBORkFOb2RlSW5kZXhTZXQsIDEyOD4gUHJlYWxsb2NhdGVkTkZBTm9kZVJh bmdlTGlzdDsKIHR5cGVkZWYgVmVjdG9yPHVpbnQzMl90LCAwLCBDb250ZW50RXh0ZW5zaW9uc092 ZXJmbG93SGFuZGxlcj4gVW5pcXVlTm9kZUxpc3Q7CiB0eXBlZGVmIFZlY3RvcjxVbmlxdWVOb2Rl TGlzdCwgMCwgQ29udGVudEV4dGVuc2lvbnNPdmVyZmxvd0hhbmRsZXI+IE5GQU5vZGVDbG9zdXJl czsKIAo= 368036 2019-04-23 08:51:57 -0700 2019-04-24 08:51:45 -0700 webkit-schar.patch v2 webkit-schar.patch text/plain 2119 wk SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI0NDUzOCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE5LTA0LTIzICBHcmVnIFYg IDxncmVnQHVucmVsZW50aW5nLnRlY2hub2xvZ3k+CisKKyAgICAgICAgRml4IC1XYysrMTEtbmFy cm93aW5nIG9uIHVuc2lnbmVkIGNoYXIgcGxhdGZvcm1zIGxpa2UgRnJlZUJTRC9hYXJjaDY0Cisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTcxNDgKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGNvbnRlbnRl eHRlbnNpb25zL0RGQUNvbWJpbmVyLmNwcDoKKyAgICAgICAgKiBjb250ZW50ZXh0ZW5zaW9ucy9O RkFUb0RGQS5jcHA6CisKIDIwMTktMDQtMjMgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNp YUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtBVEtdIEltcGxlbWVudCBBdGtDb21wb25lbnRJZmFj ZSBzY3JvbGxfdG8gbWV0aG9kcwpJbmRleDogU291cmNlL1dlYkNvcmUvY29udGVudGV4dGVuc2lv bnMvREZBQ29tYmluZXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2NvbnRlbnRl eHRlbnNpb25zL0RGQUNvbWJpbmVyLmNwcAkocmV2aXNpb24gMjQ0NTM4KQorKysgU291cmNlL1dl YkNvcmUvY29udGVudGV4dGVuc2lvbnMvREZBQ29tYmluZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC0zNyw3ICszNyw3IEBACiBuYW1lc3BhY2UgQ29udGVudEV4dGVuc2lvbnMgewogCiBjbGFzcyBE RkFNZXJnZXIgewotICAgIHR5cGVkZWYgTXV0YWJsZVJhbmdlTGlzdDxjaGFyLCB1aW50NjRfdCwg MTI4PiBDb21iaW5lZFRyYW5zaXRpb25zTXV0YWJsZVJhbmdlTGlzdDsKKyAgICB0eXBlZGVmIE11 dGFibGVSYW5nZUxpc3Q8c2lnbmVkIGNoYXIsIHVpbnQ2NF90LCAxMjg+IENvbWJpbmVkVHJhbnNp dGlvbnNNdXRhYmxlUmFuZ2VMaXN0OwogCiAgICAgZW51bSBjbGFzcyBXaGljaERGQSB7CiAgICAg ICAgIEEsCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9jb250ZW50ZXh0ZW5zaW9ucy9ORkFUb0RGQS5j cHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvY29udGVudGV4dGVuc2lvbnMvTkZBVG9E RkEuY3BwCShyZXZpc2lvbiAyNDQ1MzgpCisrKyBTb3VyY2UvV2ViQ29yZS9jb250ZW50ZXh0ZW5z aW9ucy9ORkFUb0RGQS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTQxLDkgKzQxLDkgQEAKIAogbmFt ZXNwYWNlIENvbnRlbnRFeHRlbnNpb25zIHsKIAotdHlwZWRlZiBNdXRhYmxlUmFuZ2U8Y2hhciwg TkZBTm9kZUluZGV4U2V0PiBORkFOb2RlUmFuZ2U7Ci10eXBlZGVmIE11dGFibGVSYW5nZUxpc3Q8 Y2hhciwgTkZBTm9kZUluZGV4U2V0PiBORkFOb2RlUmFuZ2VMaXN0OwotdHlwZWRlZiBNdXRhYmxl UmFuZ2VMaXN0PGNoYXIsIE5GQU5vZGVJbmRleFNldCwgMTI4PiBQcmVhbGxvY2F0ZWRORkFOb2Rl UmFuZ2VMaXN0OwordHlwZWRlZiBNdXRhYmxlUmFuZ2U8c2lnbmVkIGNoYXIsIE5GQU5vZGVJbmRl eFNldD4gTkZBTm9kZVJhbmdlOwordHlwZWRlZiBNdXRhYmxlUmFuZ2VMaXN0PHNpZ25lZCBjaGFy LCBORkFOb2RlSW5kZXhTZXQ+IE5GQU5vZGVSYW5nZUxpc3Q7Cit0eXBlZGVmIE11dGFibGVSYW5n ZUxpc3Q8c2lnbmVkIGNoYXIsIE5GQU5vZGVJbmRleFNldCwgMTI4PiBQcmVhbGxvY2F0ZWRORkFO b2RlUmFuZ2VMaXN0OwogdHlwZWRlZiBWZWN0b3I8dWludDMyX3QsIDAsIENvbnRlbnRFeHRlbnNp b25zT3ZlcmZsb3dIYW5kbGVyPiBVbmlxdWVOb2RlTGlzdDsKIHR5cGVkZWYgVmVjdG9yPFVuaXF1 ZU5vZGVMaXN0LCAwLCBDb250ZW50RXh0ZW5zaW9uc092ZXJmbG93SGFuZGxlcj4gTkZBTm9kZUNs b3N1cmVzOwogCg==