COMP: Fail the build on an undeclared external function - #81
Merged
Merged
Conversation
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.
Member
Author
|
The commit messages in this range were rewritten to remove trailers that do not belong in permanent history: This PR's commits on the rewritten
The SHA recorded above by GitHub is from the pre-rewrite history and no longer resolves. |
This was referenced Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes a function defined without a prior declaration fail the build, under both
FSLSTYLEsettings. Green on arrival; the one thing standing in the way was four undeclared pigz writers, fixed in the first commit.-Wmissing-declarationshas 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 reachedmasterwith 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-uninitializedinfslio.c:936under recent GCC. Waiting for that would leave this uncovered indefinitely, and-DCMAKE_C_FLAGS=-Werror=missing-declarationsneeds 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
doPigzanddoPigz2were the only offenders on theFSLSTYLEpath — 4 errors acrossnifti1_io.candnifti2_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
doPigz2in the NIFTI-1 library is byte-identical to thedoPigzdirectly 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 behindcmake/exported_symbols_linux.txtdoes not define it.collect_exported_symbols.shreports 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=ONFSLSTYLE=OFFEach 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:
Also checked with
-Wall -Wextraon 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.