WTF::String should use one of unsigned or size_t instead of mixed |
||||
Issue descriptionIn WTF::String, character index and length are in |unsigned| but |find()| and family return |size_t|. To avoid 64-bit to 32-bit truncation warning and mixing of |unsigned| and |size_t| in callers, we should use one of them in |WTF::String|.
,
Oct 24 2016
I'd vote for size_t, this was flagged by the compiler as well when I was compiling with -Wconversion as well.
,
Oct 24 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d0fd86cf833cd7f35eea342f15c827516938d67 commit 9d0fd86cf833cd7f35eea342f15c827516938d67 Author: Dave Tapuska <dtapuska@chromium.org> Date: Fri Jul 27 17:14:32 2018 Introduce wtf_size_t which is typedef for uint32_t. WTF vector and strings cannot be larger than UINT_MAX so change the indexing, length and find methods to return wtf_size_t. The desire to change this to the maximum size and not align it with std::vector which returns size_t is such that callers actually know the desired maximum size. One example is PaintChunks are currently using size_t indicies but the size of PaintChunks can be reduced by 8 bytes (moving two indicies to be 32 bit from 64 bit on 64 bit platforms). This change is a minimal change. Progressively all the call sites will be fixed such that they are using wtf_size_t. https://chromium-review.googlesource.com/c/chromium/src/+/1128178 is a prototype of all the fixes but is to large to commit by itself since approximately 1100 files need to be fixed. BUG= 652586 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I77db5df1313e6ab92497d4a811204707d5419c53 Reviewed-on: https://chromium-review.googlesource.com/1145045 Commit-Queue: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Yuta Kitamura <yutak@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#578684} [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/public/platform/DEPS [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/public/platform/web_vector.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/css/css_value.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/css/parser/css_parser_token_range.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/fetch/multipart_parser.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/html/forms/date_time_edit_element.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/html/forms/html_select_element.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/html/parser/html_parser_idioms.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/html/parser/html_parser_idioms.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/html/parser/xss_auditor.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/svg/properties/svg_list_property_helper.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/core/svg/svg_string_list.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/bindings/to_v8.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/graphics/paint/paint_chunk.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/heap/gc_info.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/heap/heap_allocator.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/heap/heap_test.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/heap/persistent.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/heap/threading_traits.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/heap/trace_traits.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/shared_buffer.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/web_icon_sizes_parser.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/deque.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/deque_test.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/doubly_linked_list.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/forward.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/hash_map.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/hash_set.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/hash_set_test.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/not_found.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/ref_vector.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/atomic_string.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/cstring.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/cstring.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/string_builder.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/string_concatenate.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/string_impl.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/string_impl.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/string_utf8_adaptor.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/wtf_string.cc [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/text/wtf_string.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/vector.h [modify] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/vector_test.cc [add] https://crrev.com/9d0fd86cf833cd7f35eea342f15c827516938d67/third_party/blink/renderer/platform/wtf/wtf_size_t.h
,
Jul 30
Dave, do you have plans to land the mojo/public/cpp/bindings/array_traits_wtf_vector.h part of https://chromium-review.googlesource.com/c/chromium/src/+/1128178 soon? The CL above has broken the GCC build because the templates don't match: In file included from ../../mojo/public/cpp/bindings/array_traits_span.h:11, from ../../mojo/public/cpp/bindings/lib/serialization.h:13, from gen/net/interfaces/host_resolver_service.mojom-blink.h:25, from gen/net/interfaces/host_resolver_service.mojom-blink.cc:15: ../../mojo/public/cpp/bindings/array_traits.h: In instantiation of ‘struct mojo::ArrayTraits<WTF::Vector<mojo::StructPtr<net::interfaces::blink::IPEndPoint> > >’: ../../mojo/public/cpp/bindings/lib/template_util.h:74:30: required from ‘struct mojo::internal::EnsureTypeIsComplete<mojo::ArrayTraits<WTF::Vector<mojo::StructPtr<net::interfaces::blink::IPEndPoint> > > >’ ../../mojo/public/cpp/bindings/lib/serialization_util.h:146:27: required from ‘struct mojo::internal::HasGetBeginMethod<mojo::ArrayTraits<WTF::Vector<mojo::StructPtr<net::interfaces::blink::IPEndPoint> > >, const WTF::Vector<mojo::StructPtr<net::interfaces::blink::IPEndPoint> > >’ ../../mojo/public/cpp/bindings/lib/array_serialization.h:470:74: required from ‘struct mojo::internal::Serializer<mojo::ArrayDataView<net::interfaces::IPEndPointDataView>, const WTF::Vector<mojo::StructPtr<net::interfaces::blink::IPEndPoint> > >’ ../../mojo/public/cpp/bindings/lib/serialization_forward.h:43:16: required from ‘void mojo::internal::Serialize(InputUserType&&, Args&& ...) [with MojomType = mojo::ArrayDataView<net::interfaces::IPEndPointDataView>; InputUserType = const WTF::Vector<mojo::StructPtr<net::interfaces::blink::IPEndPoint> >&; Args = {mojo::internal::Buffer*&, mojo::internal::Array_Data<mojo::internal::Pointer<net::interfaces::internal::IPEndPoint_Data> >::BufferWriter*, const mojo::internal::ContainerValidateParams*, mojo::internal::SerializationContext*&}; typename std::enable_if<(! mojo::internal::IsOptionalWrapper<InputUserType>::value)>::type* <anonymous> = 0]’ gen/net/interfaces/address_list.mojom-shared.h:115:90: required from ‘static void mojo::internal::Serializer<net::interfaces::AddressListDataView, MaybeConstUserType>::Serialize(MaybeConstUserType&, mojo::internal::Buffer*, net::interfaces::internal::AddressList_Data::BufferWriter*, mojo::internal::SerializationContext*) [with MaybeConstUserType = mojo::StructPtr<net::interfaces::blink::AddressList>]’ ../../mojo/public/cpp/bindings/lib/serialization_forward.h:43:16: required from ‘void mojo::internal::Serialize(InputUserType&&, Args&& ...) [with MojomType = net::interfaces::AddressListDataView; InputUserType = mojo::StructPtr<net::interfaces::blink::AddressList>&; Args = {mojo::internal::Buffer*&, net::interfaces::internal::AddressList_Data::BufferWriter*, mojo::internal::SerializationContext*}; typename std::enable_if<(! mojo::internal::IsOptionalWrapper<InputUserType>::value)>::type* <anonymous> = 0]’ gen/net/interfaces/host_resolver_service.mojom-blink.cc:96:64: required from here ../../mojo/public/cpp/bindings/array_traits.h:72:17: error: static assertion failed: Cannot find the mojo::ArrayTraits specialization. Did you forget to include the corresponding header file? static_assert(internal::AlwaysFalse<T>::value, ^~~~~~~~
,
Jul 30
Can you let me know if https://chromium-review.googlesource.com/c/chromium/src/+/1154990 fixes it for you?
,
Jul 30
It does, thank you!
,
Jul 30
Good, sorry about that.
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab027ae0b4f60b704322f4cb291270412eca1390 commit ab027ae0b4f60b704322f4cb291270412eca1390 Author: Dave Tapuska <dtapuska@chromium.org> Date: Mon Jul 30 15:15:11 2018 Fix GCC build issue with WTF Vector change Follow up from https://chromium.googlesource.com/chromium/src.git/+/9d0fd86cf833cd7f35eea342f15c827516938d67 Adjust mojo template. It built correctly with clang but failed to build with GCC. BUG= 652586 , 819294 TBR=jam@chromium.org Change-Id: I985bf855d5fce1f2c774123a8f6103e97aedb9f6 Reviewed-on: https://chromium-review.googlesource.com/1154990 Commit-Queue: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com> Cr-Commit-Position: refs/heads/master@{#579040} [modify] https://crrev.com/ab027ae0b4f60b704322f4cb291270412eca1390/mojo/public/cpp/bindings/array_traits_wtf_vector.h
,
Sep 14
|
||||
►
Sign in to add a comment |
||||
Comment 1 by esprehn@chromium.org
, Oct 21 2016