Update base/third_party/icu/icu_utf.{cc,h} |
||
Issue description
We’ve got a slightly modified copy of third_party/icu/source/common/utf_impl.cpp and some other ICU typedefs and macros at base/third_party/icu/icu_utf.{cc,h}. Our copy provides base_icu::utf8_nextCharSafeBody(), some typedefs in the base_icu namespace, and some macros namespaced to CBU*. These are used by some things in base.
1. Do we still need this distinct copy, or can we get rid of it and let base use third_party/icu directly? I’m assuming that the historical prohibition against icu in base (while allowing it in base_unittests and base_i18n) is still in effect.
2. If we still do need this distinct copy, should we update it? There have been changes to utf8_nextCharSafeBody(), for example, which we use via our CBU8_NEXT() macro.
,
Oct 26 2017
Thanks. This came up for me in the context of testing with GCC 7.2. I found that this file triggered the -Wimplicit-fallthrough warning, new in GCC 7.0, enabled by -Wextra. Annotations were added to ICU to deal with this same warning for Clang at https://ssl.icu-project.org/trac/changeset/39702, although I don’t hit this in Clang because -Wimplicit-fallthrough is not enabled by default (or with -Wextra) there. I’m happy to add the annotation where needed (it’s actually just mini_chromium’s copy right now), but since I noticed that there were other changes to this code, I thought I should ask you whether we should sync this. Note that other recent changes to ICU have substantially overhauled this function to the point that it no longer uses the case/switch construct on the trunk or in the ICU 60 RC at all.
,
Nov 1 2017
Perhaps, http://bugs.icu-project.org/trac/ticket/13311 has brought most of changes.
,
Nov 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/mini_chromium/+/dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec commit dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec Author: Mark Mentovai <mark@chromium.org> Date: Mon Nov 06 19:47:15 2017 Use ICU 60.1 as the basis for base/third_party/icu This matches Chromium’s https://chromium-review.googlesource.com/c/753783. This involves a subtle behavior change: illegal UTF-8 subsequences may now decode to more U+FFFD (replacement character) characters than previously. More information about this behavior change: https://sourceforge.net/p/icu/mailman/message/35990833/ https://ssl.icu-project.org/trac/changeset/40455 https://ssl.icu-project.org/trac/ticket/13311 Bug: 777950 Change-Id: I9481b3850b43f255b7b6e798bf954509641e42ef Reviewed-on: https://chromium-review.googlesource.com/755095 Reviewed-by: Robert Sesek <rsesek@chromium.org> [modify] https://crrev.com/dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec/base/third_party/icu/README.chromium [modify] https://crrev.com/dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec/base/third_party/icu/icu_utf.h [modify] https://crrev.com/dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec/base/third_party/icu/LICENSE [modify] https://crrev.com/dd0c3e9680ae3c4c22f2221a2a75e48dd4a562ec/base/third_party/icu/icu_utf.cc
,
Nov 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62ab790a0c8c7f510395476cc2ffa682e3806173 commit 62ab790a0c8c7f510395476cc2ffa682e3806173 Author: Mark Mentovai <mark@chromium.org> Date: Mon Nov 06 22:26:09 2017 Use ICU 60.1 as the basis for base/third_party/icu This involves a subtle behavior change, which will also be picked up in //third_party/icu when it’s upgraded to ICU 60.1: illegal UTF-8 subsequences may now decode to more U+FFFD (replacement character) characters than previously. Test expectations are adjusted accordingly. More information about this behavior change: https://sourceforge.net/p/icu/mailman/message/35990833/ https://ssl.icu-project.org/trac/changeset/40455 https://ssl.icu-project.org/trac/ticket/13311 Bug: 777950 , 766816 Change-Id: Ie7724fe627f61fbab941a5013c2ae2c821bb45fe Reviewed-on: https://chromium-review.googlesource.com/753783 Reviewed-by: Jungshik Shin <jshin@chromium.org> Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Mark Mentovai <mark@chromium.org> Cr-Commit-Position: refs/heads/master@{#514257} [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/base/strings/utf_offset_string_conversions_unittest.cc [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/base/strings/utf_string_conversions_unittest.cc [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/base/third_party/icu/LICENSE [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/base/third_party/icu/README.chromium [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/base/third_party/icu/icu_utf.cc [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/base/third_party/icu/icu_utf.h [modify] https://crrev.com/62ab790a0c8c7f510395476cc2ffa682e3806173/url/url_canon_unittest.cc
,
Nov 6 2017
,
Jan 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b57f780f61ed3b19445a8c37e4b312a04cfbfb2 commit 7b57f780f61ed3b19445a8c37e4b312a04cfbfb2 Author: Mostyn Bramley-Moore <mostynb@vewd.com> Date: Mon Jan 08 22:59:04 2018 don't include ICU license twice in chrome://credits Mark one copy of ICU as NOT_SHIPPED. Followup to https://chromium-review.googlesource.com/753783 Bug: 777950 , 766816 Change-Id: I86e6ba3253edb4535212dbbf148a04f97167d3d6 Reviewed-on: https://chromium-review.googlesource.com/854838 Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com> Reviewed-by: Mark Mentovai <mark@chromium.org> Cr-Commit-Position: refs/heads/master@{#527797} [modify] https://crrev.com/7b57f780f61ed3b19445a8c37e4b312a04cfbfb2/base/third_party/icu/README.chromium |
||
►
Sign in to add a comment |
||
Comment 1 by js...@chromium.org
, Oct 25 2017