-
-
Notifications
You must be signed in to change notification settings - Fork 206
fix: Fix uninitialized variable warnings in R 4.5 on Windows #2527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…arnings Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
krlmlr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: Why does the implementation of R_igraph_read_graph_ncol() change now? If it is drift that existed before, fix Stimulus definition as part of this PR so that the implementation remains unchanged.
…raph_lgl Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Fixed in commit 8d9372e. The Stimulus definitions in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Fixes R 4.5 (Windows) uninitialized-variable warnings in generated R↔C interface code by extending Stimulus type mappings and correcting Stimulus function signatures so regenerated wrappers include the proper input conversions and outputs.
Changes:
- Add missing Stimulus RC type definitions for
DOUBLEandERRORto generate proper input conversion code. - Fix Stimulus function signatures for
igraph_read_graph_ncolandigraph_read_graph_lglby restoring the missingOUT GRAPH graphparameter. - Regenerate
src/rinterface.cto include the missing argument conversions (e.g., forigraph_almost_equals,igraph_cmp_epsilon,igraph_strerror).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/stimulus/types-RC.yaml | Adds new RC type definitions intended to drive correct generated conversion code. |
| tools/stimulus/functions-R.yaml | Restores missing OUT GRAPH graph parameters to prevent wrapper drift. |
| src/rinterface.c | Regenerated wrappers now include explicit input conversion statements. |
tools/stimulus/types-RC.yaml
Outdated
| INCONV: | ||
| IN: |- | ||
| IGRAPH_R_CHECK_INT(%I%); | ||
| %C% = (igraph_error_t) INTEGER(%I%)[0]; |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERROR input conversion uses INTEGER(%I%)[0], but IGRAPH_R_CHECK_INT() (via R_check_int_scalar) assumes a numeric (REALSXP) scalar and many wrappers pass numbers. Using INTEGER() here will fail or read invalid memory when igraph_errno is a numeric. Use REAL(%I%)[0] (consistent with the existing INTEGER type conversion) or change the check/conversion pair to consistently handle INTSXP.
| %C% = (igraph_error_t) INTEGER(%I%)[0]; | |
| %C% = (igraph_error_t) REAL(%I%)[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 56c570d. Changed ERROR type to use REAL(%I%)[0] instead of INTEGER(%I%)[0], consistent with how R_check_int_scalar internally accesses values via REAL().
src/rinterface.c
Outdated
| /* Convert input */ | ||
|
|
||
| IGRAPH_R_CHECK_INT(igraph_errno); | ||
| c_igraph_errno = (igraph_error_t) INTEGER(igraph_errno)[0]; |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IGRAPH_R_CHECK_INT(igraph_errno) validates igraph_errno as a numeric scalar (it reads with REAL() internally), but the subsequent conversion uses INTEGER(igraph_errno)[0]. This will break when igraph_errno is a numeric (REALSXP), which is what the check expects. Convert using REAL(igraph_errno)[0] (cast to igraph_error_t) or adjust the check to validate INTSXP before using INTEGER().
| c_igraph_errno = (igraph_error_t) INTEGER(igraph_errno)[0]; | |
| c_igraph_errno = (igraph_error_t) REAL(igraph_errno)[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 56c570d. The generated code now uses REAL(igraph_errno)[0] instead of INTEGER(igraph_errno)[0], matching the behavior of IGRAPH_R_CHECK_INT which validates numeric scalars.
krlmlr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: The review has a point. Also, I don't yet see an ERROR type in types-RR.yaml . Double-check and fix as needed.
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Fixed in commit 56c570d. Added DOUBLE and ERROR type definitions to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
R 4.5 on Windows reports uninitialized variable warnings in
rinterface.cforigraph_almost_equals,igraph_cmp_epsilon, andigraph_strerrorfunctions. The generated C code declares variables (c_a,c_b,c_eps,c_igraph_errno) but never initializes them before use.Root Cause
Stimulus code generator lacks
DOUBLEandERRORtype definitions intools/stimulus/types-RC.yamlandtools/stimulus/types-RR.yaml, causing missing input conversion code in generated C functions.Changes
DOUBLEtype definition with INCONV/OUTCONV templates mapping to Cdoubleand RREALtype in bothtypes-RC.yamlandtypes-RR.yamlERRORtype definition with INCONV in bothtypes-RC.yamlandtypes-RR.yamlIGRAPH_R_CHECK()wrapperREAL(%I%)[0](notINTEGER(%I%)[0]) for value access, consistent withR_check_int_scalarimplementation which internally usesREAL()to access numeric scalars in Rigraph_read_graph_ncolandigraph_read_graph_lglto include missingOUT GRAPH graphparameter, preventing unintended drift in generated codesrc/rinterface.candR/aaa-auto.Rvia StimulusResult
Variables are now properly initialized before use:
For
igraph_strerror, the ERROR type correctly usesREAL()for value access:The fix also ensures that
igraph_read_graph_ncolandigraph_read_graph_lglmaintain their original implementations without drift.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.