Skip to content

COMP: Fail the build on an undeclared external function - #81

Merged
hjmjohnson merged 2 commits into
masterfrom
ci/missing-declarations
Sep 22, 2026
Merged

hjmjohnson merged 2 commits into
masterfrom
ci/missing-declarations

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

Makes a function defined without a prior declaration fail the build, under both FSLSTYLE settings. Green on arrival; the one thing standing in the way was four undeclared pigz writers, fixed in the first commit.

-Wmissing-declarations has been in the project's clean set for a while, but nothing makes it fatal, and a warning in a build that passes anyway is not read. Two changes that added declarations reached master with nothing to stop the next one.

Why one flag rather than NIFTI_WARNINGS_AS_ERRORS

Turning the whole set into errors is still blocked by -Wmaybe-uninitialized in fslio.c:936 under recent GCC. Waiting for that would leave this uncovered indefinitely, and -DCMAKE_C_FLAGS=-Werror=missing-declarations needs no preparatory cleanup.

Widening to the full set later does not conflict with this: the flag is already in the clean set, so the job simply becomes redundant when that day comes.

The pigz commit, and why the symbol baseline does not move

doPigz and doPigz2 were the only offenders on the FSLSTYLE path — 4 errors across nifti1_io.c and nifti2_io.c. Both are called only from the file that defines them and appear in no header, so they are internal and are now declared that way.

The copy of doPigz2 in the NIFTI-1 library is byte-identical to the doPigz directly beneath it and nothing calls it. I verified that by extracting both bodies and diffing them with the names normalized. It is excluded from the build rather than given linkage it does not need, following the treatment used for the unused fsliolib entry points in #37.

The exported symbol set is unchanged, checked rather than assumed: this code compiles only under PIGZ, and the shared build behind cmake/exported_symbols_linux.txt does not define it. collect_exported_symbols.sh reports the same 439 symbols from 6 libraries, and the diff against the committed baseline is empty.

Verification

On Ubuntu 24.04 with GCC 13.3, the compiler the job itself uses:

FSLSTYLE=ON FSLSTYLE=OFF
first commit alone rc=0, 0 offenders rc=0, 0 offenders
branch tip rc=0, 0 offenders rc=0, 0 offenders
tests 364/364 362/362

Each commit is green on its own, so bisecting through the branch does not pass through a broken tree.

The job bites. Reverting either commit it is meant to guard fails the build on exactly the functions that commit declared, and nothing else:

revert 4473cf9 ->  nifti1_io.c:2488  no previous declaration for 'nifti_fileexists'
                   nifti2_io.c:3402  no previous declaration for 'nifti_fileexists'

revert fde848a ->  afni_xml.c:552    no previous declaration for 'axml_recur_find_xml'
                   fslio.c:317       no previous declaration for 'FslGetHdrImgNames'
                   fslio.c:1717      no previous declaration for 'FslSetIntensityScaling'

Also checked with -Wall -Wextra on top, to confirm giving the pigz writers internal linkage did not trade one diagnostic for an unused-function warning: zero of those in either configuration.

doPigz and doPigz2 are called only from the file that defines them and
appear in no header, so they are internal. Declaring them so is what
lets -Wmissing-declarations be promoted to an error on the FSLSTYLE
path, where they were the only offenders.

The copy of doPigz2 in the NIFTI-1 library is byte-identical to doPigz
beside it and nothing calls it, so it is excluded from the build rather
than given linkage it does not need.

The exported symbol set is unchanged: this code compiles only under
PIGZ, which the shared build behind the baseline does not define.
-Wmissing-declarations is already in the project's clean set, but a
warning in a build that passes anyway is not read, so two changes that
added declarations reached master with nothing to stop the next one.

Both FSLSTYLE settings are covered. The flag catches nifti_fileexists
on one side and axml_recur_find_xml, FslGetHdrImgNames and
FslSetIntensityScaling on the other, each of which reached master as a
warning nobody acted on.
@hjmjohnson
hjmjohnson marked this pull request as ready for review September 22, 2026 12:51
@hjmjohnson
hjmjohnson merged commit d5c9224 into master Sep 22, 2026
23 checks passed
@hjmjohnson
hjmjohnson deleted the ci/missing-declarations branch September 22, 2026 12:52
@hjmjohnson

Copy link
Copy Markdown
Member Author

The commit messages in this range were rewritten to remove trailers that do not belong in permanent history: Co-Authored-By: naming an AI tool, and Claude-Session: URLs that resolve for nobody. Only messages changed — the tree at the tip of master is byte-identical, and author, committer, and dates are preserved.

This PR's commits on the rewritten master:

  • 1abbd8572b COMP: Give the pigz writers internal linkage
  • b4876bf7df COMP: Make an undeclared external function fail the build

The SHA recorded above by GitHub is from the pre-rewrite history and no longer resolves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant