Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
2024-07-07 Kevin Ushey <kevinushey@gmail.com>

* inst/include/Rcpp/internal/SEXP_Iterator.h: Avoid using VECTOR_PTR
* inst/include/Rcpp/vector/Subsetter.h: Avoid using STRING_PTR
* inst/include/RcppCommon.h: Include compatibility defines
* src/barrier.cpp: Avoid using {STRING/VECTOR}_PTR
* inst/include/Rcpp/r/compat.h: Include compatibility defines

2024-06-22 Dirk Eddelbuettel <edd@debian.org>

* DESCRIPTION (Version, Date): Roll micro version
Expand Down
2 changes: 1 addition & 1 deletion inst/include/Rcpp/internal/SEXP_Iterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SEXP_Iterator {

SEXP_Iterator( ): ptr(){} ;
SEXP_Iterator( const SEXP_Iterator& other) : ptr(other.ptr){} ;
SEXP_Iterator( const VECTOR& vec ) : ptr( get_vector_ptr(vec) ){} ;
SEXP_Iterator( const VECTOR& vec ) : ptr( RCPP_VECTOR_PTR(vec) ){} ;

SEXP_Iterator& operator=(const SEXP_Iterator& other){ ptr = other.ptr ; return *this ;}

Expand Down
39 changes: 39 additions & 0 deletions inst/include/Rcpp/r/compat.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@

//
// compat.h: Rcpp R/C++ interface class library -- compatibility defines
//
// Copyright (C) 2024 Dirk Eddelbuettel, Kevin Ushey
//
// This file is part of Rcpp.
//
// Rcpp is free software: you can redistribute it and/or modify it
// under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 2 of the License, or
// (at your option) any later version.
//
// Rcpp is distributed in the hope that it will be useful, but
// WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Rcpp. If not, see <http://www.gnu.org/licenses/>.

#ifndef RCPP_R_COMPAT_H
#define RCPP_R_COMPAT_H

#include <Rversion.h>

#if R_VERSION >= R_Version(4, 4, 2)
# define RCPP_STRING_PTR STRING_PTR_RO
#else
# define RCPP_STRING_PTR STRING_PTR
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Very nice. I wondered about the need to make this conditonal on existence of the new macro or min R version.


#if R_VERSION >= R_Version(4, 4, 2)
# define RCPP_VECTOR_PTR VECTOR_PTR_RO
#else
# define RCPP_VECTOR_PTR VECTOR_PTR
#endif

#endif /* RCPP_R_COMPAT_H */
6 changes: 3 additions & 3 deletions inst/include/Rcpp/vector/Subsetter.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ class SubsetProxy {
indices.reserve(rhs_n);
SEXP names = Rf_getAttrib(lhs, R_NamesSymbol);
if (Rf_isNull(names)) stop("names is null");
SEXP* namesPtr = STRING_PTR(names);
SEXP* rhsPtr = STRING_PTR(rhs);
const SEXP* namesPtr = RCPP_STRING_PTR(names);
const SEXP* rhsPtr = RCPP_STRING_PTR(rhs);
for (R_xlen_t i = 0; i < rhs_n; ++i) {
SEXP* match = std::find(namesPtr, namesPtr + lhs_n, *(rhsPtr + i));
const SEXP* match = std::find(namesPtr, namesPtr + lhs_n, *(rhsPtr + i));
if (match == namesPtr + lhs_n)
stop("not found");
indices.push_back(match - namesPtr);
Expand Down
1 change: 1 addition & 0 deletions inst/include/RcppCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
// #define RCPP_DEBUG_MODULE_LEVEL 1

#include <Rcpp/r/headers.h>
#include <Rcpp/r/compat.h>

/**
* \brief Rcpp API
Expand Down
2 changes: 1 addition & 1 deletion src/api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ namespace Rcpp {
case BCODESXP: return "BCODESXP";
case EXTPTRSXP: return "EXTPTRSXP";
case WEAKREFSXP: return "WEAKREFSXP";
#if R_Version >= R_Version(4,4,0) // replaces S4SXP in R 4.4.0
#if R_VERSION >= R_Version(4,4,0) // replaces S4SXP in R 4.4.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one seemed a bit strange; I'm not sure if this really worked before (or, if it did, why it did)?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that looks like you spotted an 'ooops'. Thank you!

And thanks for the update. It look good. Will have to run another rev.dep run but we should be good by end of week. Fingers crossed!

case OBJSXP: return Rf_isS4(x) ? "S4SXP" : "OBJSXP"; // cf src/main/inspect.c
#else
case S4SXP: return "S4SXP";
Expand Down
14 changes: 10 additions & 4 deletions src/barrier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@
#define COMPILING_RCPP

#define USE_RINTERNALS

#include <algorithm>
#include <Rinternals.h>

#include <Rcpp/barrier.h>
#include "internal.h"
#include <algorithm>
#include <Rcpp/protection/Shield.h>
#include <Rcpp/r/compat.h>

#include "internal.h"

// [[Rcpp::register]]
SEXP get_string_elt(SEXP x, R_xlen_t i) { // #nocov start
Expand All @@ -50,7 +54,8 @@ void char_set_string_elt(SEXP x, R_xlen_t i, const char* value) {

// [[Rcpp::register]]
SEXP* get_string_ptr(SEXP x) {
return STRING_PTR(x);
// TODO: should we deprecate this?
Copy link
Member

Choose a reason for hiding this comment

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

May need a full rev.dep run to see if it is being called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we leave this code as-is for this PR, and then later test their removal in a separate PR, to see what breaks?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a plan!

return const_cast<SEXP*>(RCPP_STRING_PTR(x));
}

// [[Rcpp::register]]
Expand All @@ -65,7 +70,8 @@ void set_vector_elt(SEXP x, R_xlen_t i, SEXP value) {

// [[Rcpp::register]]
SEXP* get_vector_ptr(SEXP x) {
return VECTOR_PTR(x); // #nocov end
// TODO: should we deprecate this?
return const_cast<SEXP*>(RCPP_VECTOR_PTR(x)); // #nocov end
}

// [[Rcpp::register]]
Expand Down