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

Use variadic templates unconditionally #1367

Merged
merged 15 commits into from
Mar 21, 2025
Merged

Conversation

eddelbuettel
Copy link
Member

As described in #1366 and building on PR #1364, we can simplify the code by no longer switching based on definition of HAS_VARIADIC_TEMPLATE: only the positive case will be considered given the baseline of C++11.

So this PR removed this file by file, testing each commit locally in CI and arrived at removing the definition. A reverse dependency check is now running as well.

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
Copy link
Member Author

Don't get to do that every day 😆

image

@kevinushey
Copy link
Contributor

Don't get to do that every day 😆

image

It's beautiful!

@eddelbuettel
Copy link
Member Author

I was a little surprised it amounted to so little in the tar.gz:

edd@rob:~/git/rcpp(feature/variadic_templates)$ ls -tltr Rcpp_1.0.14.[78].tar.gz
-rw-r--r-- 1 edd edd 2972038 Mar 16 09:51 Rcpp_1.0.14.7.tar.gz
-rw-r--r-- 1 edd edd 2889287 Mar 18 20:37 Rcpp_1.0.14.8.tar.gz
edd@rob:~/git/rcpp(feature/variadic_templates)$ 

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!

@eddelbuettel
Copy link
Member Author

Been burned before so I will probably await the reverse-depends. Maybe just maybe somone relied on the define. Otherwise this is pretty easy (if volumunious): no new code, only removal of code we no longer compiled in. Seemingly no fat-fingered typo ... 🤞

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.

Nice!

eddelbuettel added a commit to RcppCore/rcpp-logs that referenced this pull request Mar 21, 2025
@eddelbuettel eddelbuettel merged commit 926b8b8 into master Mar 21, 2025
20 checks passed
@eddelbuettel eddelbuettel deleted the feature/variadic_templates branch March 21, 2025 12:34
eddelbuettel added a commit to RcppCore/rcpp-logs that referenced this pull request Mar 24, 2025
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.

3 participants