Add SimpleStringDictionary and its test

Add SimpleStringDictionary and its test.

TEST=client_test SimpleStringDictionary*.*
R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/b980a5984bcb

[Mark Mentovai, 2014-08-20 22:47:51]

This is patchcestuous!

I made minimal changes, but all of the changes I did make should be for the
better. The Breakpad version is patch set 2 so that you can easily review the
changes.

[Robert Sesek, 2014-08-21 16:55:16]

https://codereview.chromium.org/489993002/diff/20001/client/simple_string_dic...
File client/simple_string_dictionary.h (right):

https://codereview.chromium.org/489993002/diff/20001/client/simple_string_dic...
client/simple_string_dictionary.h:81: : map_(map),
I'm not sure these clang-format changes are actually improvements.

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
File client/simple_string_dictionary.h (right):

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
client/simple_string_dictionary.h:89: DCHECK(other.key_size == key_size);
DCHECK_EQ

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
client/simple_string_dictionary.h:238: typedef NonAllocatingMap<256, 256, 64>
SimpleStringDictionary;
Since you've named this file simple_string_dictionary.h and renamed the test,
why not rename the template class and use default template parameters instead.

[Mark Mentovai, 2014-08-22 14:05:26]

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
File client/simple_string_dictionary.h (right):

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
client/simple_string_dictionary.h:89: DCHECK(other.key_size == key_size);
rsesek wrote:
> DCHECK_EQ

You would think so, but then in debug mode, you get

Undefined symbols for architecture x86_64:
  "crashpad::SimpleStringDictionary<10ul, 10ul, 10ul>::key_size", referenced
from:
      crashpad::SimpleStringDictionary<10ul, 10ul,
10ul>::operator=(crashpad::SimpleStringDictionary<10ul, 10ul, 10ul> const&) in
client_test.simple_string_dictionary_test.o

(for num_entries and value_size too). This is because the part of the logging
system that prints out the values of the two sides passes them by const
reference, and the reference winds up assuming that storage has been allocated
for the constants, but there has been no explicit instantiation of these
constants. There’s no way to do the explicit instantiation without forecasting
what values people will use as template parameters.

However, I don’t think that these checks are actually necessary. The language
already treats a template class parameter as though it had the same template
parameters as the template class that it’s found in, and this is a distinct and
unconvertible type from template instantiations with any different template
parameters. (This is why you can’t use the
std::basic_string<>::basic_string(const std::basic_string&) constructor to
initialize a std::string from a std::wstring.)

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
client/simple_string_dictionary.h:238: typedef NonAllocatingMap<256, 256, 64>
SimpleStringDictionary;
rsesek wrote:
> Since you've named this file simple_string_dictionary.h and renamed the test,
> why not rename the template class and use default template parameters instead.

Are you OK with having to instantiate it as SimpleStringDictionary<> with an
empty <> then?

[Robert Sesek, 2014-08-22 17:04:56]

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
File client/simple_string_dictionary.h (right):

https://codereview.chromium.org/489993002/diff/40001/client/simple_string_dic...
client/simple_string_dictionary.h:238: typedef NonAllocatingMap<256, 256, 64>
SimpleStringDictionary;
On 2014/08/22 14:05:26, Mark Mentovai wrote:
> rsesek wrote:
> > Since you've named this file simple_string_dictionary.h and renamed the
test,
> > why not rename the template class and use default template parameters
instead.
> 
> Are you OK with having to instantiate it as SimpleStringDictionary<> with an
> empty <> then?

I think so, yes. Or you could provide another typedef. I just think it's weird
that the bulk of this file is NonAllocatingMap but it's named for a single line
typedef. The only reason I did that in breakpad is because renaming files in
breakpad is painful.

[Mark Mentovai, 2014-08-22 17:49:56]

Like this.

[Robert Sesek, 2014-08-22 17:51:38]

I'd LGTM that. This wasn't codecestuous at all.

[Mark Mentovai, 2014-08-22 17:52:31]

Committed patchset #5 manually as b980a5984bcb (presubmit successful).