Skip to content
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

Cast size_t to uint for Rf_error #143

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Cast size_t to uint for Rf_error #143

merged 2 commits into from
Jun 25, 2024

Conversation

weshinsley
Copy link
Contributor

Speculative fix for the issue Keith raised -
See https://mrc-ide.myjetbrains.com/youtrack/issue/mrc-5482/odin.dust-Rferror-type-warning
or teams programming chat...

@weshinsley weshinsley requested review from richfitz and plietar June 19, 2024 11:52
inst/support.hpp Outdated
@@ -76,7 +76,7 @@ void user_check_array_dim(cpp11::sexp x, const char *name,
for (size_t i = 0; i < N; ++i) {
if (dim[(int)i] != dim_expected[i]) {
Rf_error("Incorrect size of dimension %d of '%s' (expected %d)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Rf_error("Incorrect size of dimension %d of '%s' (expected %d)",
Rf_error("Incorrect size of dimension %zu of '%s' (expected %d)",

Given that i has type size_t, %zu should be the portable way of printing it. If you do that then there's no need for a static_cast.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively define i as an int in the for loop, then the cast in the line above can also be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I've gone for %zu as the minimal change - I'm not sure the reason for size_t over int here, but let's keep that and just fix the print warning...

@weshinsley weshinsley requested a review from plietar June 21, 2024 08:33
@weshinsley weshinsley merged commit 7efb72e into master Jun 25, 2024
8 checks passed
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.

2 participants