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

"variadic templates" capability is misreported #1329

Open
Enchufa2 opened this issue Sep 4, 2024 · 2 comments
Open

"variadic templates" capability is misreported #1329

Enchufa2 opened this issue Sep 4, 2024 · 2 comments

Comments

@Enchufa2
Copy link
Member

Enchufa2 commented Sep 4, 2024

I see:

> Rcpp:::capabilities()["variadic templates"]
variadic templates 
             FALSE

This is checked here:

Rcpp/src/api.cpp

Lines 174 to 178 in dfa585d

#ifdef HAS_VARIADIC_TEMPLATES
LOGICAL(cap)[0] = TRUE;
#else
LOGICAL(cap)[0] = FALSE;
#endif

But HAS_VARIADIC_TEMPLATES is never set (note that the defines are commented out):

// Check C++0x/11 features
#if defined(__INTEL_COMPILER)
#if __cplusplus >= 201103L
#define RCPP_USING_CXX11
#if __INTEL_COMPILER >= 1210
// #define HAS_VARIADIC_TEMPLATES
#endif
#if __INTEL_COMPILER >= 1100
#define HAS_STATIC_ASSERT
#endif
#endif
#elif defined(__clang__)
#if __cplusplus >= 201103L
#define RCPP_USING_CXX11
#if __has_feature(cxx_variadic_templates)
// #define HAS_VARIADIC_TEMPLATES
#endif
#if __has_feature(cxx_static_assert)
#define HAS_STATIC_ASSERT
#endif
#endif
#elif defined(__GNUC__)
#ifdef __GXX_EXPERIMENTAL_CXX0X__
#if GCC_VERSION >= 40300
// #define HAS_VARIADIC_TEMPLATES
#define HAS_STATIC_ASSERT
#endif
#endif
#if GCC_VERSION >= 40800 && __cplusplus >= 201103L
#define RCPP_USING_CXX11
#endif
#endif

@Enchufa2
Copy link
Member Author

Enchufa2 commented Sep 5, 2024

I traced this code back to 270c856 that comes from this discussion. Those three lines ended up commented out after this message, that reports some issues with such feature enabled. However:

  1. That is a long time ago, like gcc 4.7 time ago.
  2. All the uses of HAS_VARIADIC_TEMPLATES in the current codebase are in the #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11) checks added by @andrjohns as part of his PR.
  3. This means that everything that defining such a macro enables is already enabled.
  4. And it's already on CRAN and well tested.

So uncommenting these 3 lines has no impact beyond correctly reporting this capability. Do you want me to open a PR? Or should we kindly ask @andrjohns to enable such defines as part of #1328, and then maybe also simplify the above checks by dropping the || defined(RCPP_USING_CXX11) part?

@eddelbuettel
Copy link
Member

I like both cleanup aspects. We can surely remove the commented-out and unused bit, and also amplifying the actual usefulness of the usage seems like a good idea. @andrjohns is closest to the metal here. Any thoughts?

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

No branches or pull requests

2 participants