Improve error messages for bad SocketParams.

Improve error messages for bad SocketParams.
Make it a compile-time error instead of link-time.
Add a check to make sure you don't try to use it with a pointer.
BUG=41016
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49823

[willchan no longer on Chromium, 2010-04-16 23:49:11]

http://codereview.chromium.org/1604042/diff/1/3
File net/socket/client_socket_pool.h (right):

http://codereview.chromium.org/1604042/diff/1/3#newcode116
net/socket/client_socket_pool.h:116: value = false,
I'm deliberately ignoring the style guide here in the naming of |value|, since
the traits should match other tr1 style traits.

[willchan no longer on Chromium, 2010-04-28 16:53:04]

Ping

On 2010/04/16 23:49:11, willchan wrote:
> http://codereview.chromium.org/1604042/diff/1/3
> File net/socket/client_socket_pool.h (right):
> 
> http://codereview.chromium.org/1604042/diff/1/3#newcode116
> net/socket/client_socket_pool.h:116: value = false,
> I'm deliberately ignoring the style guide here in the naming of |value|, since
> the traits should match other tr1 style traits.

[jar (doing other things), 2010-04-28 18:31:01]

I'm not competent enough at reading/reviewing these templates.  If you're down
in MTV, I'd be glad to be educated, but otherwise, you need to find someone that
is more up on this stuff.

I'd also argue that templates are cool... but pretty unreadable without a LOT of
commentary about intent.  Fundamentally, templates are easy to write, BECAUSE
you have the intent in your mind.   I'd suggest that the level of commenting on
templates should be much higher than "ordinary" C++ code.

http://codereview.chromium.org/1604042/diff/1/2
File base/template_util.h (right):

http://codereview.chromium.org/1604042/diff/1/2#newcode14
base/template_util.h:14: static const T value = v;
FWIW: I'm surprised you can do a static initializer for a member.  MSVC allows
this sort of thing (outside of templates), but gcc used to give an error.

[davidben, 2010-06-15 17:54:36]

To continue the conversation from
http://codereview.chromium.org/2864001/show

http://codereview.chromium.org/1604042/diff/1/3
File net/socket/client_socket_pool.h (right):

http://codereview.chromium.org/1604042/diff/1/3#newcode134
net/socket/client_socket_pool.h:134: value = true,                              
                          \
Probably should be consistent with template_util.h and use a static const bool
value = true. Or maybe better just

template <typename PoolType, typename SocketParams>
struct IsValidSocketParamsForPool
: public base::false_type {}

with the macro emitting

template<>
struct IsValidSocketParamsForPool<pool_type, socket_params>
: public base::true_type {}

(I presume the enum isn't used in tr1 to give the value the right type.)

http://codereview.chromium.org/1604042/diff/1/4
File net/socket/client_socket_pool_base_unittest.cc (right):

http://codereview.chromium.org/1604042/diff/1/4#newcode473
net/socket/client_socket_pool_base_unittest.cc:473: &ignored, NULL,
kDefaultPriority, params, NULL);
This hunk no longer applies, but resolving it is trivial.

http://codereview.chromium.org/1604042/diff/1/4#newcode492
net/socket/client_socket_pool_base_unittest.cc:492: &ignored, NULL,
kDefaultPriority, params, NULL);
This hunk no longer applies, but resolving it is trivial.

[willchan no longer on Chromium, 2010-06-15 19:03:47]

Thanks for the comments.

http://codereview.chromium.org/1604042/diff/1/3
File net/socket/client_socket_pool.h (right):

http://codereview.chromium.org/1604042/diff/1/3#newcode134
net/socket/client_socket_pool.h:134: value = true,                              
                          \
On 2010/06/15 17:54:36, David Benjamin wrote:
> Probably should be consistent with template_util.h and use a static const bool
> value = true. Or maybe better just

Good suggestion, done!

> 
> template <typename PoolType, typename SocketParams>
> struct IsValidSocketParamsForPool
> : public base::false_type {}
> 
> with the macro emitting
> 
> template<>
> struct IsValidSocketParamsForPool<pool_type, socket_params>
> : public base::true_type {}
> 
> (I presume the enum isn't used in tr1 to give the value the right type.)

Correct.

[davidben (use chromium.org), 2010-06-15 19:07:44]

LGTM.