Restrict the 1-argument ArrayView constructor to types with .size() and .data()

Restrict the 1-argument ArrayView constructor to types with .size() and .data()

Before this change, the argument still needed those methods, but not
having them resulted in a compilation error. Now, it results in this
constructor being removed from the overload set.

This currently makes no difference, but I'm about to publish a CL that
breaks without this.

Committed: https://crrev.com/bd4317263a2aed4310f99c60fca540e41a996ec9
Cr-Commit-Position: refs/heads/master@{#14068}

[kwiberg-webrtc, 2016-09-03 14:46:16]

kwiberg@webrtc.org changed reviewers:
+ ossu@webrtc.org, tommi@webrtc.org

[tommi, 2016-09-03 14:56:47]

lgtm

https://codereview.webrtc.org/2312473002/diff/1/webrtc/base/array_view.h
File webrtc/base/array_view.h (right):

https://codereview.webrtc.org/2312473002/diff/1/webrtc/base/array_view.h#newc...
webrtc/base/array_view.h:43: static constexpr bool value = (sizeof(Test<DS>(0))
== 1);
nit: == sizeof(SizeOne) to help readers connect the dots

[kwiberg-webrtc, 2016-09-03 15:56:43]

New patch set uploaded.

https://codereview.webrtc.org/2312473002/diff/1/webrtc/base/array_view.h
File webrtc/base/array_view.h (right):

https://codereview.webrtc.org/2312473002/diff/1/webrtc/base/array_view.h#newc...
webrtc/base/array_view.h:43: static constexpr bool value = (sizeof(Test<DS>(0))
== 1);
On 2016/09/03 14:56:47, tommi (webrtc) wrote:
> nit: == sizeof(SizeOne) to help readers connect the dots

I shortened it some more, with the help of std::is_same. I'd argue that it's
even more readable that way.

[tommi, 2016-09-03 21:02:25]

nice, lgtm

[ossu, 2016-09-05 09:32:27]

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
File webrtc/base/array_view_unittest.cc (right):

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
webrtc/base/array_view_unittest.cc:55: static_assert(!DS<Test3, int>::value,
"");
Surely, this should be DS<Test4, int>::value!

[kwiberg-webrtc, 2016-09-05 09:40:18]

New patch set posted.

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
File webrtc/base/array_view_unittest.cc (right):

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
webrtc/base/array_view_unittest.cc:55: static_assert(!DS<Test3, int>::value,
"");
On 2016/09/05 09:32:27, ossu wrote:
> Surely, this should be DS<Test4, int>::value!

Yes...

[ossu, 2016-09-05 09:50:44]

lgtm with nit

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
File webrtc/base/array_view_unittest.cc (right):

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
webrtc/base/array_view_unittest.cc:55: static_assert(!DS<Test3, int>::value,
"");
On 2016/09/05 09:40:18, kwiberg-webrtc wrote:
> On 2016/09/05 09:32:27, ossu wrote:
> > Surely, this should be DS<Test4, int>::value!
> 
> Yes...
Ah, so it's supposed to be false because data and size are private? Right. It's
a bit unclear that that's what's going on. I expected it to be true. Maybe just
highlight the intent with a comment. Explicitly putting private: in the class
could also work, but since it's not practically needed, it might look weird as
well.

[kwiberg-webrtc, 2016-09-05 10:11:16]

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
File webrtc/base/array_view_unittest.cc (right):

https://codereview.webrtc.org/2312473002/diff/20001/webrtc/base/array_view_un...
webrtc/base/array_view_unittest.cc:55: static_assert(!DS<Test3, int>::value,
"");
On 2016/09/05 09:50:44, ossu wrote:
> On 2016/09/05 09:40:18, kwiberg-webrtc wrote:
> > On 2016/09/05 09:32:27, ossu wrote:
> > > Surely, this should be DS<Test4, int>::value!
> > 
> > Yes...
> Ah, so it's supposed to be false because data and size are private? Right.
It's
> a bit unclear that that's what's going on. I expected it to be true. Maybe
just
> highlight the intent with a comment. Explicitly putting private: in the class
> could also work, but since it's not practically needed, it might look weird as
> well.

Added comments.

[kwiberg-webrtc, 2016-09-05 10:11:40]

The CQ bit was checked by kwiberg@webrtc.org

[kwiberg-webrtc, 2016-09-05 10:11:40]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org,
ossu@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2312473002/#ps60001 (title:
"comments")

[commit-bot: I haz the power, 2016-09-05 10:11:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-05 11:20:58]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-05 11:21:05]

Description was changed from

==========
Restrict the 1-argument ArrayView constructor to types with .size() and .data()

Before this change, the argument still needed those methods, but not
having them resulted in a compilation error. Now, it results in this
constructor being removed from the overload set.

This currently makes no difference, but I'm about to publish a CL that
breaks without this.
==========

to

==========
Restrict the 1-argument ArrayView constructor to types with .size() and .data()

Before this change, the argument still needed those methods, but not
having them resulted in a compilation error. Now, it results in this
constructor being removed from the overload set.

This currently makes no difference, but I'm about to publish a CL that
breaks without this.

Committed: https://crrev.com/bd4317263a2aed4310f99c60fca540e41a996ec9
Cr-Commit-Position: refs/heads/master@{#14068}
==========

[commit-bot: I haz the power, 2016-09-05 11:21:06]

Patchset 4 (id:??) landed as
https://crrev.com/bd4317263a2aed4310f99c60fca540e41a996ec9
Cr-Commit-Position: refs/heads/master@{#14068}