samtools / htslib

C library for high-throughput sequencing data formats
Other
783 stars 447 forks source link

Compiler pedantry #1777

Closed jkbonfield closed 2 months ago

jkbonfield commented 2 months ago

Newer clang's complain about functions declared in int func() { /* do stuff */ } style. They require an explict func(void) to be used.

header.c:2361:27: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
 2361 | sam_hrecs_t *sam_hrecs_new() {
      |                           ^
      |                            void
1 error generated.

I'm not entirely convinced with pandering to such pointless pedantry, especially as it'll go away again in newer C versions, but here it is anyway.

jkbonfield commented 2 months ago

Note this will need an additional commit to update htscodecs after https://github.com/samtools/htscodecs/pull/117 lands. Once that's merged I can add in a new commit to this (or feel free to just do it direct and push back to re-run the tests - we don't need to review that other than "it now passes").

jkbonfield commented 2 months ago

I took the liberty of just merging the htscodecs changes as frankly they're trivial to proof read and have zero code impact anyway (except for silencing -pedantic). So this PR can now also run its own tests without waiting on a series of A before B before C merges.

jmarshall commented 2 months ago

That's a pretty surprising and pointless diagnostic IMHO — maybe you folk should consider dropping ‑pedantic or adding ‑std=c23? Personally I think such a warning on function definitions encouraging people to hypercorrect their definitions like this is really just a compiler bug, which I've raised as llvm/llvm-project#90596 FWIW.

(I see that GCC also complains about this, but not by default even with ‑pedantic: you have to ask for ‑Wstrict-prototypes explicitly, and can maybe suppress it with ‑Wno-old-style-definition. Hmmm…)