Fix -Wextra

[SoapGentoo]

Fixes for when building under -Wextra, particularly for users who want to compile their Rcpp code with -Wextra.

Checklist

• Code compiles correctly
• R CMD check still passes all tests
• Preferably, new tests were added which fail without the change
• Document the changes by file in ChangeLog

[eddelbuettel]

Thanks but please see the file Contributing.md -- prior discussion is always best before throwing a PR over the fence which touches eighteen files and comes from someone we never interacted with (as far as I can tell).

[eddelbuettel]

A priori, I am sympapthetic. I have used gcc and g++ for decades, and regularly use -Wall -pedantic. Both those options are mentioned repeatedly in (somewhat normative for R) manual Writing R Extensions -- but -Wetra is not. This extra option is also described as something that

enables a set of additional compiler warnings for code constructs that are not strictly errors but are questionable, prone to bugs, or hard to avoid.

so it is a little outside the norm. We are about to release 1.1.2 in out usual 'January and July' cadence but maybe we can look into this for the release thereafter, i.e. January 2027.

[SoapGentoo]

sure, I just thought it was a rather simple change, since it doesn't touch any real logic. Do you want me to close it or leave it open and rebase it later?

[eddelbuettel]

Can we maybe decompose it into individual warnings and among the team (with between us decades of gcc experience) see which may have merit and turn them on one by one? So yes maybe this is now the issue discussion we could have had.

You appear to be associated with gentoo. Does -Wextra get turned on by default for everything over there, or is this more your personal thing?

[SoapGentoo]

-Wextra enables -Wunused-parameter and -Wdeprecated-copy-with-user-provided-copy which get triggered: both fixes are in separate commits.

This is unrelated to Gentoo, this is something I'm seeing in our corporate codebase (where we do enable -Wextra everywhere)

[eddelbuettel]

I am quite religious about -Wall -pedantic, personally speaking. But I don't think I have seen -Wextra deployed widely. Happy to move towards it. Maybe we can do it can option at a time?

[SoapGentoo]

Sure, the biggest problem for the main codebase (not in the headers) is the [[Rcpp::export]] parsing: it would have to account for unused parameters there too, which is obviously tricky. Would you want to add code such that

// [[Rcpp::export]]
Rcpp::List foo(std::string a, std::string /*b*/) { ... }

works?

[eddelbuettel]

There should be none in the Rcpp package proper i.e. in src/ and inst/include. It will matter for client packages consuming Rcpp::compileAttributes() output. Here this only affects internal use in ephemeral files (e.g. unit tests) that should get not tickle the warning.

[kevinushey]

In principle I'm okay with this change, but IMHO it's not really about fixing -Wextra warnings, it's about whether we're willing to commit to doing this from now and into the future.

That probably also implies us building and testing Rcpp with -Wextra on CI, so we don't regress this in the future.

I also worry that there's some amount of divergence in what -Wextra means, based on the compiler used, its version, and so on -- since IIUC the set of compiler flags in -Wextra is itself a moving target.

My two cents...

[SoapGentoo]

yeah, just double-checked, they're fine (I saw the -Wcast-function-type-mismatch ones, those are obviously not fixable within Rcpp without R actually fixing them first). How do you want me to proceed?

@kevinushey oh it is, but -Wall is a moving target too, so...

[eddelbuettel]

.... which is a flag that R itself will complain about as 'not portable' (cf other discussion on -pedantic etc in WRE) if we added it to src/Makevars.

[Enchufa2]

It would have been nice to discuss these as separate issues. My two cents about the copy constructor.

In general, I prefer to solve warnings rather than just silencing them. Adding a default constructor does the latter, because it's something the compiler already does for us.

Having a custom assignment operator but not a copy constructor is what triggers the warning. So the question for me is whether we need a custom copy operator too, or if the assignment operator is needed at all. Correct me if I'm wrong, but if we remove it, the compiler generates a default one that delegates to the base class, which already handles the assignment nicely (i.e. shares the underlying SEXP, which has the tzone attribute already).

[Enchufa2]

As for -Wunused-parameter,

• Is this the result of a comprehensive search and therefore probably all cases? Or just the ones that popped as warnings in your pipeline?
• The [[Rcpp::export]] parser is pretty complex as it is, I don't think we want to support comments or attributes like [[maybe_unused]] for that matter. Unused parameters typically arise in the context of template specializations, but when a user exports a function to R, typically the function requires all the parameters. And in the very few edge cases, and if they want to silence those warnings, there are other ways to do so.

[SoapGentoo]

• This was from running GCC 15 and Clang 21 on the codebase with -Wextra.
• Agreed, I wouldn't try and put this in the parser either.