types: provide __SIZEOF_{INT{8,16,32,64},TIME,TIME32}_T

[rlibby]

Summary

Suggested by: kib

Test Plan

make tinderbox

[emaste]

Seems fine to me. Is it worth an exp-run?

[imp]

sys/sys/_types.h

70

So this is silly. You are adding a lot of extra overhead for a
#define __SIZEOF_INT64_T 8
because you've already hard-coded the 8 into the checks for which of these sizeof to use.

If you are going to be silly here, you should test for SIZEOF_LONG_LONG == 8 here, since otherwise you'll get the wrong answer.

[jrtc27]

sys/sys/_types.h

70

Or just put the defines next to the corresponding typedefs, like was done with time_t?

[rlibby]

sys/sys/_types.h

70

Yes, I followed the structure of the block above which does the typedefs. I thought having the defines in their own block and following that structure would be more readable than inserting them into the block above. I agree that testing __SIZEOF_LONG_LONG__ == 8 carries out that logic more fully, and that that requires the size to be 8.

I could just set the define to 8 here and change the block above to test __SIZEOF_LONG_LONG__ == 8?

[imp]

sys/sys/_types.h

70

Or just put the defines next to the corresponding typedefs, like was done with time_t?

That would be better, likely the best solution.

I could just set the define to 8 here and change the block above to test __SIZEOF_LONG_LONG__ == 8?

So you are missing the point. Why say "if foo == 8 bar=foo else of baz==8 bar=baz" when you can say "bar=8" The extra layer of checking and obfuscation buy nothing but confusion. I think these belong where they are defined, not here.

[rlibby]

sys/sys/_types.h

70

I had thought that would be less readable, but maybe it's simplest after all.

I'm also now wondering if directly testing __SIZEOF_LONG_LONG could cause a pedantic compiler compiling for a standard before C99 to barf. Just moving the defines into the block above and not touching the #elif __SIZEOF_LONG__ == 4 test would avoid that.

[rlibby]

sys/sys/_types.h

70

I'm happy to move the defines, I'll do that.

[rlibby]

imp & jrtc27 feedback: don't repeat the long size detection logic

[rlibby]

In D56783#1300973, @emaste wrote:

Seems fine to me. Is it worth an exp-run?

Maybe? I'm not familiar with it. Is there documentation somewhere? It looks like one files a bugzilla ticket?

I wouldn't be against having that testing but I think it may be overkill for this. Do the requests get pooled?

If we do want it, it should include the child patch here too, D56401, to sys/sys/time.h. Otherwise these are just unused defines.

[emaste]

Yes, via a PR -- https://docs.freebsd.org/en/articles/committers-guide/#ports-exp-run. You can attach a patch to be applied to src (here the patch would include both the parent review and this one). It will get tested in isolation. This sometimes seems excessive and batching a few seemingly trivial changes seems like it would make sense. That said, if there are failures in that hypothetical combined exp-run it may be difficult to figure out what's responsible, especially when there may be a number of already-failing or intermittently flaky ports.

__ is reserved namespace so nothing should fail and I think it's fair if we deem the risk is low enough to not bother with an exp-run.

[imp]

In D56783#1301197, @emaste wrote:

Yes, via a PR -- https://docs.freebsd.org/en/articles/committers-guide/#ports-exp-run. You can attach a patch to be applied to src (here the patch would include both the parent review and this one). It will get tested in isolation. This sometimes seems excessive and batching a few seemingly trivial changes seems like it would make sense. That said, if there are failures in that hypothetical combined exp-run it may be difficult to figure out what's responsible, especially when there may be a number of already-failing or intermittently flaky ports.

__ is reserved namespace so nothing should fail and I think it's fair if we deem the risk is low enough to not bother with an exp-run.

Nothing should, but this is adjacent to another pattern that's widely implemented . I think the risk is low enough we needn't do an exp-run, but maybe high enough that Ryan should plan on pushing the change sometime when he'll have the time to deal with unforeseen issues quickly if we opt not to.

[kib]

I would commit this without exp run.

[rlibby]

Okay, sounds good. I'll skip the exp-run on this and plan to push it probably on Friday afternoon.

Thanks for the pointers, Ed.