[ccan] [PATCH] maskn & bitops: add new modules

[David Gibson]

On Wed, Jul 09, 2014 at 09:00:18AM -0400, Cody P Schafer wrote:
> >>> +
> >>> +unsigned fls_m1_32(uint32_t v) CONST_FUNCTION;
> >>> +unsigned clz_32(uint32_t v) CONST_FUNCTION;
> >>> +unsigned ctz_32(uint32_t v) CONST_FUNCTION;
> >>
> >> Shouldn't these be inlines?  At least the builtin versions; seems a
> >> bit silly to use the one-instruction versions when available, then
> >> wrap then in a function call.
> >>
> >
> > I end up using lto anyhow, so making the header more complicated
> > wasn't a priority.
> >
> >>> +
> >>> +#endif
> >>> diff --git a/ccan/bitops/test/run-no-builtin.c b/ccan/bitops/test/run-no-builtin.c
> >>> new file mode 100644
> >>> index 0000000..5871a1f
> >>> --- /dev/null
> >>> +++ b/ccan/bitops/test/run-no-builtin.c
> >>> @@ -0,0 +1,4 @@
> >>> +#include <ccan/bitops/bitops.h>
> >>> +#undef builtin_clz
> >>> +#undef builtin_ctz
> >>> +#include "run.h"
> >>
> >> This seems really fragile.  A rearrangement of what's in the .c versus
> >> the .h could easily mean this silently uses the builtin versions, so
> >> not testing what you're trying to test.
> >
> > But I'm not seeing an alternative that lets me avoid doing the
> > preprocessor's work it self (by manually copy-pasting the contents of
> > run.h). Do you see one?
> >
> 
> So looking at these 2 comments again:
>  - it appears that my test infra won't actually test the non-builtins
> at all (because I'm defining builtin_c*z in the .c)

Actually, I think it does, since run.c includes bitops.c, rather than
linking to it.  But the fact it's confused both of us working that out
rather illustrates the problem.

So, if you handle the possibility of builtins (including the ICCARM
ones) as HAVE_* macros from the configurator, you don't need to do
anything special, ccanlint will already test with the feature macros
disabled.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/ccan/attachments/20140710/7f90a55c/attachment.sig>