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
Unknown Object (File)
Wed, May 29, 10:48 PM
Unknown Object (File)
Wed, May 22, 4:51 AM
Unknown Object (File)
May 13 2024, 11:54 PM
Unknown Object (File)
May 8 2024, 12:54 PM
Unknown Object (File)
May 7 2024, 4:03 AM
Unknown Object (File)
May 3 2024, 4:08 AM
Unknown Object (File)
Apr 29 2024, 5:46 AM
Unknown Object (File)
Apr 28 2024, 11:32 PM

Details

Summary

Remove pre-C11 supports from stdlib

Test Plan
make buildworld
make buildkernel

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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 ↗(On Diff #132026)

This line should remain

usr.bin/lex/initscan.c
34 ↗(On Diff #132026)

Where's the end removed?

usr.bin/lex/initskel.c
474 ↗(On Diff #132026)

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 ↗(On Diff #132736)

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 ↗(On Diff #133110)

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 ↗(On Diff #133110)

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
279–282 ↗(On Diff #133535)

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)
minsoochoo0122_proton.me marked an inline comment as done.
minsoochoo0122_proton.me added inline comments.
sys/conf/kern.mk
279–282 ↗(On Diff #133535)

Make a new revision D44145

@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.