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

Remove deprecated std::(unary|binary)_function (closes #1201) #1202

Merged
merged 4 commits into from
Mar 12, 2022

Conversation

eddelbuettel
Copy link
Member

As discussed in #1201, CRAN would like us to update Rcpp to no longer trigger 'deprecated' warnings under (fairly brandnew) compilers. This PR replaces the use of std::unary_function() in two files and std::binary_function() in one.

The reverse-depends check is ongoing and as not revealed any issues yet.

  • 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, but I do wonder if we can just remove the inheritance from unary_function / binary_function altogether, since all those classes are is a bunch of typedefs. E.g. on macOS with libc++:

template <class _Arg, class _Result>
struct _LIBCPP_TEMPLATE_VIS unary_function
{
    typedef _Arg    argument_type;
    typedef _Result result_type;
};
template <class _Arg1, class _Arg2, class _Result>
struct _LIBCPP_TEMPLATE_VIS binary_function
{
    typedef _Arg1   first_argument_type;
    typedef _Arg2   second_argument_type;
    typedef _Result result_type;
};

And since we don't use those typedefs (or pass these along to functions expecting these typedefs) I think it's safe to just remove them.

@eddelbuettel
Copy link
Member Author

Yep, I hear you. And @Enchufa2 already suggested the same in #1201 (comment). I think I am going to be risk-averse here especially on a hot-fix but if someone has the energy to test this for the next regular (i.e. July) release go for it. The replacement of these two with std::function is simple enough.

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.

There's another unary_function in inst/tinytest/cpp/sugar.cpp, and then we should remove the unary-thing also from the vignettes (the sugar and FAQ vignettes contain examples with unary_function).

@eddelbuettel
Copy link
Member Author

Thanks for the heads-up. I had seen the one in the test file, but then forgot about it. Made the same conditional change. And replaced unconditional in the two markdown files. (One had not been touched in a while and Emacs decided to fiddle with a lot of whitespace so the diff looks bigger there. It isn't. GH has a toggle too for diff disply without whitespace.)

rev.dep is about half-way done and looks fine so far. I had ran mostly RcppArmadillo of late so a few build-depends need refreshing and I have a few failed-only-because-xyz-missing.

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.

GH has a toggle too for diff disply without whitespace.

Thanks, that's useful, didn't know about it.

LGTM!

@eddelbuettel
Copy link
Member Author

After the usual reverse-depends checks it got onto CRAN without hickups so merging.

@eddelbuettel eddelbuettel merged commit f12d9fd into master Mar 12, 2022
@eddelbuettel eddelbuettel deleted the bugfix/unary_binary_func_deprecation branch March 13, 2022 17:42
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.

None yet

3 participants