Page MenuHomeFreeBSD
Differential D53830

sys/stdint.h: add C23 _WIDTH macros
Needs ReviewPublic
Actions

Authored by fuz on Wed, Nov 19, 7:27 PM.
Tags
None
Referenced Files
F136864151: D53830.id.diff
Thu, Nov 20, 5:29 AM
Unknown Object (File)
Thu, Nov 20, 2:07 AM
Unknown Object (File)
Thu, Nov 20, 1:55 AM
Unknown Object (File)
Thu, Nov 20, 1:45 AM
Unknown Object (File)
Thu, Nov 20, 1:28 AM
Unknown Object (File)
Thu, Nov 20, 1:09 AM
Unknown Object (File)
Wed, Nov 19, 9:09 PM
Unknown Object (File)
Wed, Nov 19, 8:35 PM
Subscribers
brooks
emaste
jhibbits
riscv

Details

Reviewers
imp
markj
andrew
manu
Summary

The platform-dependent macros are added to the various
_stdint.h headers, those that are always the same are
added directly to _stdint.h.

We may want to move the definitions for WCHAR_* and
WINT_* out of the platform header files as those are
always the same.

Test Plan

see following DR

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 68744
Build 65627: arc lint + arc unit

Event Timeline

fuz created this revision.Wed, Nov 19, 7:27 PM
Herald added a reviewer: andrew. · View Herald TranscriptWed, Nov 19, 7:27 PM
Herald added a reviewer: andrew. · View Herald Transcript
Herald added a reviewer: manu. · View Herald Transcript
Herald added subscribers: riscv, jhibbits, emaste. · View Herald Transcript
fuz requested review of this revision.Wed, Nov 19, 7:27 PM
Harbormaster completed remote builds in B68744: Diff 166795.Wed, Nov 19, 7:28 PM
fuz mentioned this in D53831: libc/tests: add test for *_MAX, *_MIN, and *_WIDTH.Wed, Nov 19, 7:28 PM
fuz added a parent revision: D53825: sys/limits.h: add C23 _WIDTH macros.Wed, Nov 19, 7:28 PM
fuz added a child revision: D53831: libc/tests: add test for *_MAX, *_MIN, and *_WIDTH.
markj added a subscriber: brooks.Wed, Nov 19, 10:20 PM
markj added inline comments.
sys/arm64/include/_stdint.h
167

Presumably this one should be INT64_WIDTH.

I'm not sure how to handle CHERI here, but @brooks can hopefully comment on that.

brooks added a comment.Thu, Nov 20, 9:14 AM

I don't feel super strongly about this, but I'm not entirely convinced we should be defining these from first principles. I believe that compilers we care about define __*_WIDTH__ macros we could use instead and things are likely to go very badly wrong if there's disagreement.

sys/arm64/include/_stdint.h
167

Once fixed these should be correct. WIDTH is about representability not storage.

sys/riscv/include/_stdint.h
163
fuz added a comment.Thu, Nov 20, 11:30 AM
In D53830#1229971, @brooks wrote:

I don't feel super strongly about this, but I'm not entirely convinced we should be defining these from first principles. I believe that compilers we care about define __*_WIDTH__ macros we could use instead and things are likely to go very badly wrong if there's disagreement.

I agree with this. If desired, I can prepare an alternative patch set that (a) replaces the existing sys/${ARCH}/include/_types and sys/${ARCH}/include/_stdint stuff with a generic approach where possible and then (b) adds the C23 features on top. This would be an alternative approach that is more intrusive, but ultimately simpler.

sys/arm64/include/_stdint.h
167

Yes, that's a typo. Will push a fix once my arm64 machine has completed a test build (it takes a while...)

fuz updated this revision to Diff 166839.Thu, Nov 20, 12:43 PM
  • fix INTPTR_WIDTH on arm64 and riscv
Harbormaster completed remote builds in B68763: Diff 166839.Thu, Nov 20, 12:43 PM

Revision Contents
Changeset List

PathSize
sys/
arm/
include/
23 lines
arm64/
include/
23 lines
powerpc/
include/
33 lines
riscv/
include/
23 lines
sys/
29 lines
x86/
include/
33 lines

Diff 166795

View Options

sys/arm/include/_stdint.h

Loading...
View Options

sys/arm64/include/_stdint.h

Loading...
View Options

sys/powerpc/include/_stdint.h

Loading...
View Options

sys/riscv/include/_stdint.h

Loading...
View Options

sys/sys/stdint.h

Loading...
View Options

sys/x86/include/_stdint.h

Loading...