Add initializer_list constructor to base::span |
|||||
Issue description
Consider adding an initializer_list constructor to base::span so that a base::span<const T> can be implicitly constructed from an std::initializer_list<T>.
This would allow the following:
void Foo(base::span<const int> foo) {
// Use |foo|.
}
Foo({1, 2, 3, 4});
This is useful when Foo is called with various sets of values which are all constexprs and so the array allocation necessary if it took const std::vector<int>& can be avoided.
,
Dec 18 2017
Adding jdoerrie@ who has been maintaining base::span. Since this is class mirrors one being proposed for inclusion in the STL our own custom constructors may not be welcome.
,
Dec 19 2017
dcheng@ and I discussed this offline some time ago. We decided to not add a std::initializer_list<T> constructor for now, as it is not mentioned in the current standardization proposal: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0122r5.pdf I'm happy to add one in case the proposal gets updated in the future. In the meantime you could add an overload for Foo that dispatches to the span version, e.g. like this: void Foo(std::initializer_list<int> list) { Foo(base::make_span(list.begin(), list.end())); } This is the pattern we currently use for base::Value. Also see the related discussion on the corresponding base::Value CL: https://chromium-review.googlesource.com/c/chromium/src/+/623707#message-616ca8be5d5f971227fb678fdec094855b067339
,
Jan 24 2018
Marking this as WontFix for now.
,
Mar 27 2018
The std::span proposal got updated [1] and allows construction from std::initializer_list, among others. dcheng@ and I discussed adding the constructors in https://crrev.com/c/976123/9/base/containers/span.h#199. I will send out a CL that implements this change.
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82d9545a59298536b27e95096148b1bb12f80fc4 commit 82d9545a59298536b27e95096148b1bb12f80fc4 Author: jdoerrie <jdoerrie@chromium.org> Date: Tue Apr 17 13:57:48 2018 [base] Add missing methods to base::span This change implements constructors for base::span that allow the construction from a pair of pointers, std::arrays and containers supporting base::data and base::size, e.g. std::initializer_list. In addition, this change adds operator(). Bug: 788913 Change-Id: Ibc280eef1c7e47a5a27e92503dda3614ef5513a4 Reviewed-on: https://chromium-review.googlesource.com/981139 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org> Cr-Commit-Position: refs/heads/master@{#551313} [modify] https://crrev.com/82d9545a59298536b27e95096148b1bb12f80fc4/base/containers/span.h [modify] https://crrev.com/82d9545a59298536b27e95096148b1bb12f80fc4/base/containers/span_unittest.cc [modify] https://crrev.com/82d9545a59298536b27e95096148b1bb12f80fc4/base/containers/span_unittest.nc
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5ca9f13ffa33418061a326e178384d6461aa713 commit c5ca9f13ffa33418061a326e178384d6461aa713 Author: Mohsen Izadi <mohsen@chromium.org> Date: Tue Apr 17 14:44:43 2018 Revert "[base] Add missing methods to base::span" This reverts commit 82d9545a59298536b27e95096148b1bb12f80fc4. Reason for revert: compile failures on ios-simulator-xcode-clang and ios-device-xcode-clang builders: * https://uberchromegw.corp.google.com/i/chromium.mac/builders/ios-simulator-xcode-clang/builds/46343 * https://uberchromegw.corp.google.com/i/chromium.mac/builders/ios-device-xcode-clang/builds/57515 Original change's description: > [base] Add missing methods to base::span > > This change implements constructors for base::span that allow the > construction from a pair of pointers, std::arrays and containers > supporting base::data and base::size, e.g. std::initializer_list. > In addition, this change adds operator(). > > Bug: 788913 > Change-Id: Ibc280eef1c7e47a5a27e92503dda3614ef5513a4 > Reviewed-on: https://chromium-review.googlesource.com/981139 > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org> > Cr-Commit-Position: refs/heads/master@{#551313} TBR=dcheng@chromium.org,jdoerrie@chromium.org Change-Id: Ia967227caa3a24a10df2ab4c5fa4ee03dddf0f29 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 788913 Reviewed-on: https://chromium-review.googlesource.com/1014761 Reviewed-by: Mohsen Izadi <mohsen@chromium.org> Commit-Queue: Mohsen Izadi <mohsen@chromium.org> Cr-Commit-Position: refs/heads/master@{#551329} [modify] https://crrev.com/c5ca9f13ffa33418061a326e178384d6461aa713/base/containers/span.h [modify] https://crrev.com/c5ca9f13ffa33418061a326e178384d6461aa713/base/containers/span_unittest.cc [modify] https://crrev.com/c5ca9f13ffa33418061a326e178384d6461aa713/base/containers/span_unittest.nc
,
Apr 17 2018
While r551313 now allows construction a base::span<const T> from a std::initializer_list<T> via the container constructor [1], this unfortunately does not solve the issue raised in #c1, as the template deduction fails here. What is now possible is to simplify the workaround from void Foo(std::initializer_list<int> list) { Foo(base::make_span(list.begin(), list.end())); } to void Foo(std::initializer_list<int> list) { Foo(base::make_span(list)); } Smarter deduction will require a dedicated constructor overload taking a std::initializer_list, similarly to how Abseil does it [2]. However, since this constructor is not part of the official standard proposal, we shouldn't add it at this point. Marking at WontFix again. [1] http://eel.is/c++draft/span.cons#itemdecl:5 [2] https://github.com/abseil/abseil-cpp/blob/a7e522daf1ec9cda69b356472f662142dd0c1215/absl/types/span.h#L342-L344
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11201f072ca67ad3046b0720fa89840f4fd62981 commit 11201f072ca67ad3046b0720fa89840f4fd62981 Author: Jan Wilken Doerrie <jdoerrie@chromium.org> Date: Wed Apr 18 11:03:58 2018 Reland "[base] Add missing methods to base::span" This is a reland of 82d9545a59298536b27e95096148b1bb12f80fc4 Original change's description: > [base] Add missing methods to base::span > > This change implements constructors for base::span that allow the > construction from a pair of pointers, std::arrays and containers > supporting base::data and base::size, e.g. std::initializer_list. > In addition, this change adds operator(). > > Bug: 788913 > Change-Id: Ibc280eef1c7e47a5a27e92503dda3614ef5513a4 > Reviewed-on: https://chromium-review.googlesource.com/981139 > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org> > Cr-Commit-Position: refs/heads/master@{#551313} TBR=dcheng@chromium.org Bug: 788913 Change-Id: I2e02879ddd2839ce0c26c8b18e565d555e08cc7b Reviewed-on: https://chromium-review.googlesource.com/1013468 Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org> Cr-Commit-Position: refs/heads/master@{#551627} [modify] https://crrev.com/11201f072ca67ad3046b0720fa89840f4fd62981/base/containers/span.h [modify] https://crrev.com/11201f072ca67ad3046b0720fa89840f4fd62981/base/containers/span_unittest.cc [modify] https://crrev.com/11201f072ca67ad3046b0720fa89840f4fd62981/base/containers/span_unittest.nc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by pkl@chromium.org
, Dec 18 2017