Page MenuHomeFreeBSD

libc: Remove support for pre-C11 C standards
ClosedPublic

Authored by minsoochoo0122_proton.me on Dec 31 2023, 6:45 AM.
Referenced Files
F102796558: D43254.diff
Sun, Nov 17, 7:46 AM
Unknown Object (File)
Fri, Oct 25, 7:47 AM
Unknown Object (File)
Fri, Oct 25, 7:47 AM
Unknown Object (File)
Fri, Oct 25, 7:46 AM
Unknown Object (File)
Fri, Oct 25, 7:46 AM
Unknown Object (File)
Fri, Oct 25, 7:46 AM
Unknown Object (File)
Fri, Oct 25, 7:46 AM
Unknown Object (File)
Fri, Oct 25, 7:46 AM

Details

Summary

Remove pre-C11 supports from stdlib

Test Plan
make buildworld
make buildkernel

Diff Detail

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

Event Timeline

lib/libc/stdlib/div.c
43–44

This code isn't compiled for c99 and higher. Shouldn't you remove it?

lib/libc/stdlib/imaxdiv.c
38–39

Same

lib/libc/stdlib/ldiv.c
45–46

Same

lib/libc/stdlib/lldiv.c
38–39

Same

share/mk/bsd.sys.mk
23 ↗(On Diff #132026)

This line should remain

sys/conf/kern.mk
288

This line should remain

usr.bin/lex/initscan.c
34

Where's the end removed?

usr.bin/lex/initskel.c
474

Same

minsoochoo0122_proton.me edited the summary of this revision. (Show Details)

Removed stdlib division code for < C99

jhb added inline comments.
share/mk/bsd.sys.mk
14 ↗(On Diff #132029)

This is not just used by the base system, but by anything built using bsd.*.mk including various ports. Removing the ability to set CSTD needs an exp-run, not just testing buildworld.

share/mk/bsd.sys.mk
14 ↗(On Diff #132029)

To be clear, I think requiring C11 for the base system (and removing bits from libc, etc. for older C) is probably reasonable, but removing the build glue that might be used in ports is probably too aggressive.

share/mk/bsd.sys.mk
14 ↗(On Diff #132029)

Let's put a .error for the old standards and which ports break in an exprun... then we can decide what to do.

share/mk/bsd.sys.mk
14 ↗(On Diff #132029)

The only problem here is -std=c94 and -std=c95 since they are not implemented in clang.

$ clang -std=c94 main.c
error: invalid value 'c94' in '-std=c94'
note: use 'c89', 'c90', or 'iso9899:1990' for 'ISO C 1990' standard
note: use 'iso9899:199409' for 'ISO C 1990 with amendment 1' standard
note: use 'gnu89' or 'gnu90' for 'ISO C 1990 with GNU extensions' standard
note: use 'c99' or 'iso9899:1999' for 'ISO C 1999' standard
note: use 'gnu99' for 'ISO C 1999 with GNU extensions' standard
note: use 'c11' or 'iso9899:2011' for 'ISO C 2011' standard
note: use 'gnu11' for 'ISO C 2011 with GNU extensions' standard
note: use 'c17', 'iso9899:2017', 'c18', or 'iso9899:2018' for 'ISO C 2017' standard
note: use 'gnu17' or 'gnu18' for 'ISO C 2017 with GNU extensions' standard
note: use 'c2x' for 'Working Draft for ISO C2x' standard
note: use 'gnu2x' for 'Working Draft for ISO C2x with GNU extensions' standard
share/mk/bsd.sys.mk
13 ↗(On Diff #132029)

This line is removed in D43237

Add .error for c94 and c95 flags which are undefined in clang. Other flags are defined by default.

share/mk/bsd.sys.mk
15 ↗(On Diff #132736)

I think you misunderstand what the old code does here. The old code maps "c94" and "c95" to the supported string you now use in the error message. I think you should leave this part unchanged. If we want to deprecate older versions (which should be a separate commit and run via an exp-run in ports), we should add explicit .error's for older standards, e.g.:

.if ${CSTD} == "c89" || ${CSTD} == "c90"
.error "Only c99 or later is supported"
...

However, the old code doesn't pass -cstd=c94 to either clang or GCC today, so I don't think the argument for removing the current bits is valid.

sys/conf/kern.mk
279

It might be nice to explicitly .error here for older C standards, but that isn't required. We might want a comment here saying what our actual minimum version is (even in today's sources we probably should be saying we require gnu99 at minimum). It might be worth fixing this file as cleanup commit first to document our current de facto minimum (gnu99) and removing the support in this file for older versions. This file is only used for the kernel and associated modules so doesn't need the same legacy support as bsd.sys.mk.

jrtc27 requested changes to this revision.Jan 29 2024, 9:12 PM
jrtc27 added a subscriber: jrtc27.
jrtc27 added inline comments.
usr.bin/lex/initscan.c
6

This is generated; the code you delete comes from contrib/flex/src/flexint.h (via flex.skl). It will just reappear on regeneration. So please drop this.

usr.bin/lex/initskel.c
1

Same comment as initscan.c.

This revision now requires changes to proceed.Jan 29 2024, 9:12 PM

Commit message should probably be qualified with "for kernel compilation"?

Commit message should probably be qualified with "for kernel compilation"?

Originally this was both kernel and user space compilation, and that is why there are changes for libc, too. Modifying share/mk/bsd.sys.mk can brake port builds, but this revision has all the changes we need for both kernel and userland compilation.

sys/conf/kern.mk
278–279

I would commit this part separately from the other changes

minsoochoo0122_proton.me retitled this revision from Remove support for pre-C11 C standards to libc: Remove support for pre-C11 C standards.Feb 28 2024, 11:43 PM
minsoochoo0122_proton.me edited the summary of this revision. (Show Details)

@jrtc27 Can you review the changes?

This looks ok to me now.

The commit log though can say "Remove pre-C99 support" though since the blocks are all for pre-C99.

Hey John do you want to land this or should I?

This revision was not accepted when it landed; it landed in state Needs Review.Apr 12 2024, 7:40 PM
This revision was automatically updated to reflect the committed changes.