Summary: | [aarch64] clang Wc++11-narrowing for unsigned char in WebCore/contentextensions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | wk <wk> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, ap, commit-queue, mcatanzaro, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Local Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Other | ||||||||
Attachments: |
|
Description
wk@unrelenting.technology
2019-04-21 08:08:56 PDT
Comment on attachment 367910 [details] 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? Comment on attachment 367910 [details] 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. You'll also need a ChangeLog entry. Running Tools/Scripts/prepare-ChangeLog will give you a template to fill in. 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). Created attachment 368036 [details]
webkit-schar.patch v2
added changelog entry
Comment on attachment 368036 [details] 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 :/ Comment on attachment 368036 [details] webkit-schar.patch v2 Clearing flags on attachment: 368036 Committed r244591: <https://trac.webkit.org/changeset/244591> All reviewed patches have been landed. Closing bug. |