build: allow building with recent C compilers #72
Conversation
Before this commit, nimutils (and therefore Chalk [1]) could not be
built with recent stable releases of Clang and GCC, as some warnings now
default to an error. This affected Clang 15.0.0 (2022-09-06) or later,
and GCC 14.1 (2024-05-07) or later, and:
- Made it harder to build Chalk outside of a container during
development. Simply writing the options from this commit on the
command line after `nimble build` was not sufficient, because that
wouldn't forward the options to the command used to compile e.g.
this repo's `strcontainer.c` or `switchboard.c`.
- Would prevent certain OS or compiler version bumps for the builder
image.
For now, simply disable these errors. Some are in md4c.h, and others
will be resolved when we move to libcon4m.
As background, see e.g.:
- The breaking changes section of the release notes for Clang 16, which
contains [2]:
The `-Wimplicit-function-declaration` and `-Wimplicit-int`
warnings now default to an error in C99, C11, and C17. As of C2x,
support for implicit function declarations and implicit int has
been removed, and the warning options will have no effect.
Specifying `-Wimplicit-int` in C89 mode will now issue warnings
instead of being a noop.
-Wincompatible-function-pointer-types` now defaults to an error in
all C language modes.
- The release notes for Clang 15 [3]:
The `-Wint-conversion` warning diagnostic for implicit int <->
pointer conversions now defaults to an error in all C language
modes.
- The porting guide for GCC 14 [4]:
The initial ISO C standard and its 1999 revision removed support
for many C language features that were widely known as sources of
application bugs due to accidental misuse. For backwards
compatibility, GCC 13 and earlier diagnosed use of these features
as warnings only. Although these warnings have been enabled by
default for many releases, experience shows that these warnings
are easily ignored, resulting in difficult to diagnose bugs. In
GCC 14, these issues are now reported as errors, and no output
file is created, providing clearer feedback to programmers that
something is wrong.
#### Implicit function declarations (`-Werror=implicit-function-declaration`)
It is no longer possible to call a function that has not been
declared. In general, the solution is to include a header file
with an appropriate function prototype. Note that GCC will perform
further type checks based on the function prototype, which can
reveal further type errors that require additional changes.
#### Using pointers as integers and vice versa (`-Werror=int-conversion`)
GCC no longer treats integer types and pointer types as equivalent
in assignments (including implied assignments of function
arguments and return values), and instead fails the compilation
with a type error.
#### Type checking on pointer types (`-Werror=incompatible-pointer-types`)
GCC no longer allows implicitly casting all pointer types to all
other pointer types. This behavior is now restricted to the void *
type and its qualified variations.
- The Gentoo wiki page on Modern C porting [5].
[1] crashappsec/chalk#290
[2] https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes
[3] https://releases.llvm.org/15.0.0/tools/clang/docs/ReleaseNotes.html
[4] https://gcc.gnu.org/gcc-14/porting_to.html#warnings-as-errors
[5] https://wiki.gentoo.org/wiki/Modern_C_porting
A bump from Nim 2.0.0 to Nim 2.0.8 only bumps Nim itself, and consequently Nim itself should no longer generate C code for which newer C compilers produce errors by default. However, before this PR, The bump from our The context there is: |
ee7
changed the title
|
No objections to actually merge this since its happening anyhow, but what is the level of effort required to actually address these in their root cause? |
Turn these back into warnings, even when the C compiler produces errors by default, rather than suppressing the message completely.
|
For visibility: these C compilation errors have now blocked aarch64 macOS from inclusion in the latest Chalk binaries (0.4.9, 2024-07-30). My understanding is that this should only be due to GitHub updating the runner's default version of Clang, although the release notes make no mention of this. However, I have noticed over the years that sometimes-surprising major version bumps are made in the GitHub runner images, and in particular I do recall Clang major version bumps when pinned to
Likely not enormous. However, at least some are from the external markdown parser md4c, and others will disappear once libcon4m is ready and chalk has integrated it. For libcon4m, I added the most recent stable releases of clang and gcc to CI, and the only
That is, none of the errors handled by this PR occur in libcon4m, and libcon4m CI enforces that. In case it was preferable, I've added commit c8e6ef2 so that this PR merely downgrades these errors to warnings, rather than disabling the diagnostic entirely. |
|
merging to include as part of the next patch release to fix mac os arm builds |
TL;DR: We should be able to build Chalk outside a container without specifying extra compiler options. But this is currently not possible when recent C compilers (Clang: within the last couple of years; GCC: within the last few months) are installed, due to C compilation errors.
Fixing this would make building Chalk:
Right now, there's a significant subset of developers who would see errors if they cloned the Chalk repo and ran
nimble build.I suggest that we add three
passClines in nimutils todisable these errorsdowngrade these errors to warnings, and move on.Before this PR, nimutils (and therefore Chalk, see crashappsec/chalk#290) could not be built with recent stable releases of Clang and GCC, as some warnings now default to an error. This affected Clang 15.0.0 (2022-09-06) or later, and GCC 14.1 (2024-05-07) or later, and:
Made it harder to build Chalk outside of a container during development. Simply writing the options from this PR on the command line after
nimble buildwas not sufficient, because that wouldn't forward the options to the command used to compile e.g. this repo'sstrcontainer.corswitchboard.c.Would prevent certain OS or compiler version bumps for the builder image.
For now, simply
disable these errorsdowngrade these errors to warnings. Some are inmd4c.h, and others will be resolved when we move to libcon4m.As background, see e.g. the breaking changes section of the release notes for Clang 16:
and release notes for Clang 15:
and the porting guide for GCC 14:
and the Gentoo wiki page on Modern C porting.