fix: init.c prototypes + header comment, and finish inst/WORDLIST (PUNY-qguubpin, PUNY-ddljtzrv) - #96
Merged
Conversation
…er (PUNY-qguubpin) Three declarations used () instead of (void). In C that means "unspecified arguments", not "no arguments", so clang emitted -Wstrict-prototypes for each on every build: _punycoder_backend_info_cpp, _punycoder_normalization_unicode_version_cpp, _punycoder_unicode_versions_cpp. A clean rebuild now compiles init.c with no warnings. Also replace the header comment, which claimed the file is generated by Rcpp's compileAttributes() and must not be edited by hand. That is false and misleading in a way that can cost a runtime failure: compileAttributes() rewrites src/RcppExports.cpp and R/RcppExports.R only, neither of which carries R_registerRoutines(), so this file is the sole registration point and is maintained by hand. With useDynLib(punycoder, .registration = TRUE), a new [[Rcpp::export]] whose CallEntries row is missing fails at runtime as "symbol not found", not at compile time -- and the old comment told the reader registration was automatic. The remaining init.c warning under -pedantic builds (enumeration types with a fixed underlying type are a C23 extension) originates in R's own R_ext/Boolean.h and is not actionable here. Verified with rm -f src/*.o && R CMD INSTALL . (clean rebuild, zero warnings) and the full suite against the installed build: 529 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…NY-ddljtzrv) spell_check_package() now reports zero flagged words, down from 34 before this release cycle's cleanup. The other 16 were Fiberplane issue slugs, removed from NEWS.md in f42d483 rather than suppressed here. Added: ADR, Dependabot, LDH, NFD, OSS, TRE, UCD, bytewise, canonicalizer, hardcoded, mispredicting, normalizations, pre, selectable, transcode, transcoded, trie, unshipped. The file is LC_ALL=C sorted; verified that convention before editing and re-sorted, so the diff is 18 pure insertions with nothing reordered. Note this only silences the local spelling check. CRAN runs its own aspell and does not read inst/WORDLIST; the current R CMD check --as-cran produces no misspelled-words NOTE regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
2 participants
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.
Two backlog items from the
/crangate review. Independent, one commit each.src/init.c—-Wstrict-prototypes(PUNY-qguubpin)Three zero-argument entry points were declared
()rather than(void). In Cthat means unspecified arguments, so clang warned on each, every build. Now
compiles clean.
While in the file: the header comment claimed it was generated by
compileAttributes()and must not be hand-edited. That's false, and false ina costly direction —
compileAttributes()only rewritessrc/RcppExports.cppand
R/RcppExports.R, neither of which carriesR_registerRoutines(). This isthe sole registration point and is hand-maintained. Given
useDynLib(punycoder, .registration = TRUE), a new[[Rcpp::export]]missingits
CallEntriesrow fails at runtime as "symbol not found" — while the oldcomment told the reader registration was automatic. Replaced with one that says
what the file actually requires.
The remaining
-pedanticwarning (C23 extension,Rboolean) comes from R'sown
R_ext/Boolean.hand is not actionable here.inst/WORDLIST— the last 18 words (PUNY-ddljtzrv)spell_check_package()now reports zero flagged words, down from 34. Theother 16 were Fiberplane issue slugs, removed from
NEWS.mdin f42d483 ratherthan suppressed here. The file is
LC_ALL=Csorted; I verified that conventionfirst, so the diff is 18 pure insertions with nothing reordered.
Only silences the local check — CRAN runs its own aspell and ignores
inst/WORDLIST. The currentR CMD check --as-cranhas no misspelled-wordsNOTE either way.
Verification
Clean rebuild per AGENTS.md (
rm -f src/*.o; R CMD INSTALL .) — zerowarnings from
init.c. Full suite run against the installed build ratherthan
devtools::test(), to avoid leaving-O0objects insrc/:529 pass, 0 fail.
spell_check_package()clean.🤖 Generated with Claude Code