[ccan] [PATCH 2/4] order_functions: Scalar comparison functions

[Rusty Russell]

David Gibson <david at gibson.dropbear.id.au> writes:
> On Tue, Jun 02, 2015 at 10:36:41AM +0930, Paul 'Rusty' Russell wrote:
>> David Gibson <david at gibson.dropbear.id.au> writes:
>> > Extend the order_functions module to provide simple, standard, comparison
>> > callbacks for scalar types (i.e. integer and floating point types).
>> >
>> > In addition to the usual variants comparing a plain scalar, this also
>> > provides helper macros to construct a suitably typed callback and context
>> > pointer to order structures by a specified scalar field.
>> 
>> This is *so awesome!*, I think many of my funcs can be
>> trivially replaced by these.
>> 
>> Two things:
>> 1) Should we just expose the macro in the header, and be done?
>>    Hmm, I'd say not, since asort() won't be inlined anyway.
>
> Um.. I'm not entirely sure what you're getting at here.
>
>>    But maybe s/SCALAR_ORDER/ORDER_SCALAR/ for namespace reasons
>>    and leave that in the header, with a /* You can build your own! */
>>    comment?
>
> I wondered about that.  The wrinkle is that the macro generates
> exported functions, whereas in the most common case of creating your
> own they'll want to be static.
>
> Obviously the macro will only work for types which have C builtin <, >
> and == implementations.  That doesn't actually leave a lot of options
> that aren't already covered in the module - basically just typedef
> aliases to integer types.  I'm not sure how worthwhile leaving the
> macro in the header just for that is.

OK, I buy that.

>> 2) I'd prefer, despite the extra typing, to have the explicit
>>    declarations in the header anyway.  It's just far more readable.
>
> By this I'm assuming you mean get rid of the _DECL_ONAME_* macros and
> just fully expand them in the header?

Yes.  The same argument could go for the body, I guess, since TAGS et al
will like that more.  But it's marginal.

Thanks,
Rusty.