Skip to content

Conversation

@eddelbuettel
Copy link
Member

This PR addresses warnings seen when building with clang++-14 -Wall -pedantic -Wconversion -Wno-sign-conversion as done on the CRAN 'M1mac' box. It is a fairly simple set of casts. It seems to make sense to fold this into the pending 1.0.10 even though we otherwise dislike making very last minute changes...

Checklist

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

Copy link
Contributor

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@Enchufa2 Enchufa2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@eddelbuettel
Copy link
Member Author

Thank you both!

@eddelbuettel eddelbuettel merged commit 39669cb into master Jan 9, 2023
@eddelbuettel eddelbuettel deleted the feature/clang_conversion_warnings branch January 9, 2023 16:03
@eddelbuettel
Copy link
Member Author

I will need a one-line follow-up as the change in attributes.cpp now triggers a warning under g++-12. Can't win 😉

ccache g++ -I"/usr/share/R/include" -DNDEBUG -I../inst/include/     -fpic  -g -O2 -Wall -pipe -pedantic  -c attributes.cpp -o attributes.o
attributes.cpp: In member function ‘std::string Rcpp::attributes::SourceFileAttributesParser::parseSignature(size_t)’:
attributes.cpp:1624:38: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘R_xlen_t’ {aka ‘long int’} [-Wsign-compare]
 1624 |         for (size_t i = lineNumber; i<lines_.size(); i++) {
      |                                     ~^~~~~~~~~~~~~~                  
ccache g++ -I"/usr/share/R/include" -DNDEBUG -I../inst/include/     -fpic  -g -O2 -Wall -pipe -pedantic  -c barrier.cpp -o barrier.o

@eddelbuettel eddelbuettel mentioned this pull request Jan 9, 2023
4 tasks
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.

4 participants