On 24 Jul 2014, at 03:48, Trevor Saunders <tsaund...@mozilla.com> wrote:
> On Thu, Jul 24, 2014 at 02:00:24AM +0200, Oleg Endo wrote:
>> On Wed, 2014-07-23 at 11:37 +0200, Richard Biener wrote:
>>> On Fri, Jul 18, 2014 at 3:08 AM, Trevor Saunders <tsaund...@mozilla.com>
>>> wrote:
>>>> On Thu, Jul 17, 2014 at 06:36:31AM +0200, Andi Kleen wrote:
>>>>> On Wed, Jul 16, 2014 at 10:40:53PM -0400, Trevor Saunders wrote:
>>>>>>
>>>>>>> + public:
>>>>>>> +
>>>>>>> + /* Start incremential hashing, optionally with SEED. */
>>>>>>> + void begin (hashval_t seed = 0)
>>>>>>> + {
>>>>>>> + val = seed;
>>>>>>
>>>>>> why isn't this the ctor?
>>>>>
>>>>> It's standard for hash classes to have explicit begin()/end().
>>>>> All the existing ones I've seen work this way.
>>>>
>>>> I only know of one vaguelly similar thing
>>>> http://mxr.mozilla.org/mozilla-central/source/mfbt/SHA1.h#37 which
>>>> doesn't do that, and a bunch of people doing something doesn't
>>>> necessarily mean it makes sense. Now there may be a good reason it
>>>> does make sense, but unless these other people need begin() to be
>>>> fallible I don't see it.
>>>
>>> I agree with Trevor here. Please make begin() the constructor.
>>> Btw, what will be the way to plug in an alternative hash function?
>>> That is, there doesn't seem to be a separation of interface
>>> and implementation in your patch (like with a template or a base-class
>>> you inherit from).
>>
>> Isn't that what boost / std hash is actually doing? Maybe something
>> like the attached example could be an improvement?
>
> I don't really see why that makes it any easier to plug in a different
> hash algorithm
typedef incremental_hash<my_hash, my_hash_combiner> my_inc_hash;
^^^^^^^
replace with a different class
True, it's not any 'easier' than replacing any other name in the code.
It makes it easier to mix different type dependent hash functions/
algorithms, though. The way I understood it, Richard asked how to replace
the hash function for the incremental hash class.
>
>> As with boost / std hash, the hash function is a template functor that
>> can be specialized on various types. The 'incremental_hash' simply
>> plugs together a hash value combiner and a hash function. Types that
>> are not implemented by the hash function (in this example) are caught at
>> compile time, since implicit conversions do not take place. However, it
>> should also possible to write hash functions with a bunch of operator ()
>> overloads to utilize implicit conversions.
>
> at that point it seems like its simpler and equivelent to just overload
> a global function.
AFAIR, there are some cases which are difficult to handle with overloads
(implicit conversions etc). It could be a freestanding template
function that is specialized for the types in question.
>
>> One advantage of this construct is that new types can be added along
>> with hash function specializations, without the need to touch any of the
>> existing hashing facilities.
>>
>> I don't know how/whether this would fit with the already existing hash
>> map stuff (hash-table.h, hash-map.h). It seems those don't support user
>> specified hash functions.
>
> they do, see e.g. graphite-htab.h
Aha. I see.
Cheers,
Oleg
