Skip to content
Open
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
77 changes: 77 additions & 0 deletions .github/PATCHSTACK.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# The patch stack

This is a fork of [r-lib/cpp11](https://github.com/r-lib/cpp11) that carries a
handful of changes upstream does not ship.
It is maintained as a *patch stack*:
one branch per change,
each a single commit on top of upstream,
and one branch that is upstream plus all of them.

## The branches

- **`main`** is a 1:1 mirror of `r-lib/cpp11`'s `main`.
The [Pull app](https://pull.git.ci/) hard-resets it on every sync
([`pull.yml`](pull.yml)),
so a commit made here is discarded the next time upstream moves.
Never commit to it.
- **`fork`** is the default branch and the one to install:
`main` plus every patch, squashed in lexicographic branch order.
It is rebuilt from scratch on each sync — never commit to it either,
and never merge into it.
- **`a-fork-infra`**, **`b-*`**, **`f-*`** are the patch branches,
each based on `main`.
`a-` is this fork's own infrastructure (it sorts first, so it lands first),
`b-` is a bug fix, `f-` is a feature.
[`patchstack-sync.yml`](workflows/patchstack-sync.yml) picks up
`b-*` and `f-*` by glob,
so a working branch under any other name is left alone.

## The sync

[`patchstack-sync.yml`](workflows/patchstack-sync.yml) runs
[krlmlr/patchstack](https://github.com/krlmlr/patchstack) nightly,
and on demand from the Actions tab.
Each run replays every patch branch onto the current `main`,
squashes them into a fresh `fork` in branch order,
and pushes the lot atomically.

A patch that no longer applies is left at its old commit
and dropped from that run's `fork`,
so one broken patch never blocks the others —
resolve it by rebasing that branch onto `main` by hand and pushing it.
A patch whose change has landed upstream replays to nothing;
its branch is deleted and the disposition recorded in `refs/notes/patchstack`.

## Adding a patch

```bash
git fetch origin
git switch -c f-my-change origin/main
# ... one commit ...
git push -u origin f-my-change
```

The next sync folds it into `fork`.
Keep it to a single commit where you can:
the stack is easier to read,
and the squash into `fork` is what everyone consumes anyway.

Each patch is a candidate for upstream.
Opening a pull request from its branch against `r-lib/cpp11`
costs nothing here — the branch stays exactly where the sync expects it —
and a merged patch cleans itself up on the following run.

## Installing

```r
pak::pak("krlmlr/cpp11")
```

`fork` is the default branch, so this installs the whole stack.
It is also built by
[krlmlr.r-universe.dev](https://krlmlr.r-universe.dev),
which is the faster route:

```r
install.packages("cpp11", repos = c("https://krlmlr.r-universe.dev", getOption("repos")))
```
26 changes: 26 additions & 0 deletions .github/pull.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Configuration for the "Pull" GitHub App, which keeps this fork's `main` in sync
# with the upstream r-lib/cpp11 repository.
#
# Pointers:
# - App / install / manage: https://pull.git.ci/
# - Source: https://github.com/wei/pull
# - Configuration reference: https://github.com/wei/pull/blob/master/docs/CONFIGURATIONS.md
# - Trigger a manual sync: https://pull.git.ci/process/krlmlr/cpp11
#
# Notes:
# - This file must live on the fork's default branch (`fork`) for Pull to read it.
# - `main` is a 1:1 mirror of r-lib/cpp11's `main`, so `hardreset` is the only
# correct merge method: any commit made on it here is discarded on the next sync.
# Nothing may be committed to `main` -- the patch branches carry this fork's work.
# - `fork` has no rule on purpose: it is not a mirror but `main` plus the squashed
# patch stack, and .github/workflows/patchstack-sync.yml rebuilds it once Pull
# has moved `main` forward.
# - Deleting this file would not disable Pull: without a configuration it hard-resets
# the fork's default branch from upstream, which is exactly what `fork` must not do.

version: "1"

rules:
- base: main
upstream: r-lib:main
mergeMethod: hardreset
6 changes: 4 additions & 2 deletions .github/workflows/R-CMD-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
# usethis::use_github_action("check-standard") will install it.
on:
push:
branches: [main, master]
# `fork` is this fork's integration branch, rebuilt from `main` plus the
# patch stack; checking it is what says the stack still builds.
branches: [main, master, fork]
pull_request:
branches: [main, master]
branches: [main, master, fork]

name: R-CMD-check

Expand Down
63 changes: 63 additions & 0 deletions .github/workflows/patchstack-sync.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
name: Patchstack sync

# Rebuilds `fork` -- this repository's default branch -- as `main` plus the
# squashed patch stack, replaying every patch branch onto `main` first.
#
# `main` is a 1:1 mirror of r-lib/cpp11, hard-reset by the Pull app
# (see .github/pull.yml), so it is the upstream this sync tracks and there is no
# second remote to fetch. The patch branches are the `b-*` (bug fix) and `f-*`
# (feature) branches; everything else in this repository is left alone.
#
# The push needs a token that may write workflow files, because `fork` carries
# this file and .github/pull.yml through the `a-fork-infra` patch. GITHUB_TOKEN
# may not, and it only matters when their content changes -- a sync that leaves
# them untouched goes through with the default token. Set PATCHSTACK_TOKEN to a
# fine-grained PAT with Contents: read and write and Workflows: read and write
# to make the exception unnecessary.

on:
schedule:
# Off the hour: runs scheduled at :00 queue behind everyone else's.
- cron: "23 4 * * *"
workflow_dispatch:
repository_dispatch:
types: [main-updated]

concurrency:
group: patchstack-sync
cancel-in-progress: false

permissions:
contents: read

jobs:
sync:
name: Rebuild the fork branch
runs-on: ubuntu-latest
timeout-minutes: 20

permissions:
contents: write

steps:
- name: Check out this repository
uses: actions/checkout@v6
with:
# The replay needs every patch branch's history; blobs come on demand.
fetch-depth: 0
filter: blob:none
token: ${{ secrets.PATCHSTACK_TOKEN || github.token }}

- name: Check out patchstack
uses: actions/checkout@v6
with:
repository: krlmlr/patchstack
path: .patchstack
persist-credentials: false

- name: Sync
uses: ./.patchstack
with:
main_branch: fork
upstream_branch: main
patch_glob: "b-* f-*"
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# cpp11 (development version)

* `cpp_register()` now also picks up a `{package}_types.h` or `{package}_types.hpp` in `src/include/`, letting a package keep that header private instead of installing it from `inst/include/`.

# cpp11 0.5.5

* Fixed an issue where `cpp11::stop()` and `cpp11::warning()` calls with the same template instantiation could cause a crash on some systems (#491, #295).
Expand Down
23 changes: 17 additions & 6 deletions R/register.R
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ cpp_register <- function(
funs <- get_registered_functions(all_decorations, "cpp11::register", quiet)

package <- desc::desc_get("Package", file = file.path(path, "DESCRIPTION"))
package <- sub("[.]", "_", package)
package <- gsub("[.]", "_", package)

cpp_functions_definitions <- generate_cpp_functions(funs, package)

Expand Down Expand Up @@ -114,17 +114,28 @@ cpp_register <- function(
)
}

pkg_types_name <- paste0(package, c("_types.h", "_types.hpp"))

pkg_types <- c(
file.path(path, "src", paste0(package, "_types.h")),
file.path(path, "src", paste0(package, "_types.hpp")),
file.path(path, "inst", "include", paste0(package, "_types.h")),
file.path(path, "inst", "include", paste0(package, "_types.hpp"))
file.path(path, "src", pkg_types_name),
file.path(path, "src", "include", pkg_types_name),
file.path(path, "inst", "include", pkg_types_name)
)

# `src/cpp11.cpp` is generated next to the `src/` copies and compiled from
# `src/`, so a header in `src/include/` is included through that directory.
# A package keeping its own headers private this way then needs no include
# flag for them, and none of the other two locations changes.
pkg_types_include <- c(
pkg_types_name,
file.path("include", pkg_types_name),
pkg_types_name
)

pkg_types_exist <- file.exists(pkg_types)
if (any(pkg_types_exist)) {
extra_includes <- c(
sprintf('#include "%s"', basename(pkg_types[pkg_types_exist])),
sprintf('#include "%s"', pkg_types_include[pkg_types_exist]),
extra_includes
)
}
Expand Down
29 changes: 19 additions & 10 deletions R/vendor.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
#' code until you run `cpp_vendor()` again.
#'
#' @inheritParams cpp_register
#' @param date The date recorded in the `vendored on:` header of each vendored
#' file. Defaults to the current date; pass a fixed date to make vendoring
#' reproducible.
#' @param overwrite If `TRUE`, an existing vendored copy is removed first
#' instead of raising an error.
#' @return The file path to the vendored code (invisibly).
#' @export
#' @examples
Expand All @@ -30,18 +35,22 @@
#'
#' # cleanup
#' unlink(dir, recursive = TRUE)
cpp_vendor <- function(path = ".") {
cpp_vendor <- function(path = ".", date = Sys.Date(), overwrite = FALSE) {
new <- file.path(path, "inst", "include", "cpp11")

if (dir.exists(new)) {
stop(
"'",
new,
"' already exists\n * run unlink('",
new,
"', recursive = TRUE)",
call. = FALSE
)
if (overwrite) {
unlink(new, recursive = TRUE)
} else {
stop(
"'",
new,
"' already exists\n * run unlink('",
new,
"', recursive = TRUE)",
call. = FALSE
)
}
}

dir.create(new, recursive = TRUE, showWarnings = FALSE)
Expand All @@ -56,7 +65,7 @@ cpp_vendor <- function(path = ".") {
cpp11_header <- sprintf(
"// cpp11 version: %s\n// vendored on: %s",
cpp11_version,
Sys.Date()
as.Date(date)
)

files <- list.files(current, full.names = TRUE)
Expand Down
49 changes: 49 additions & 0 deletions cpp11test/src/test-external_pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,53 @@ context("external_pointer-C++") {
uniq.reset();
expect_true(deleted == true);
}

test_that("external_pointer preserves attributes when moved (issue #308)") {
// Test move constructor
{
int* value = new int(42);
cpp11::external_pointer<int> p(value);

// Set an attribute on the external pointer
Rf_setAttrib(p, R_ClassSymbol, Rf_mkString("test_class"));

// Verify attribute exists before move
SEXP class_attr = Rf_getAttrib(p, R_ClassSymbol);
expect_true(class_attr != R_NilValue);

// Move the external pointer using move constructor
cpp11::external_pointer<int> p_moved = std::move(p);

// Verify attribute is preserved after move
SEXP class_attr_after = Rf_getAttrib(p_moved, R_ClassSymbol);
expect_true(class_attr_after != R_NilValue);
expect_true(strcmp(CHAR(STRING_ELT(class_attr_after, 0)), "test_class") == 0);

// Clean up
delete p_moved.release();
}

// Test move assignment operator
{
int* value1 = new int(1);
cpp11::external_pointer<int> p1(value1);

// Set an attribute on p1
Rf_setAttrib(p1, R_ClassSymbol, Rf_mkString("test_class"));

// Create p2 with nullptr (no memory leak)
cpp11::external_pointer<int> p2(nullptr);

// Move assign p1 to p2
p2 = std::move(p1);

// Verify attribute is preserved after move assignment
SEXP class_attr_after = Rf_getAttrib(p2, R_ClassSymbol);
expect_true(class_attr_after != R_NilValue);
expect_true(strcmp(CHAR(STRING_ELT(class_attr_after, 0)), "test_class") == 0);

// Clean up
delete p2.release();
}
}
}
5 changes: 3 additions & 2 deletions inst/include/cpp11/declarations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ T& unmove(T&& t) {
SEXP err = R_NilValue; \
char buf[CPP11_ERROR_BUFSIZE] = ""; \
try {
#define END_CPP11 \
#define END_CPP11_EX(RET) \
} \
catch (cpp11::unwind_exception & e) { \
err = e.token; \
Expand All @@ -57,4 +57,5 @@ T& unmove(T&& t) {
} else if (err != R_NilValue) { \
R_ContinueUnwind(err); \
} \
return R_NilValue;
return RET;
#define END_CPP11 END_CPP11_EX(R_NilValue)
19 changes: 15 additions & 4 deletions inst/include/cpp11/external_pointer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ class external_pointer {

external_pointer(SEXP data) : data_(valid_type(data)) {}

external_pointer(pointer p, bool use_deleter = true, bool finalize_on_exit = true)
: data_(safe[R_MakeExternalPtr]((void*)p, R_NilValue, R_NilValue)) {
external_pointer(pointer p, bool use_deleter = true, bool finalize_on_exit = true,
SEXP prot = R_NilValue)
: data_(safe[R_MakeExternalPtr]((void*)p, R_NilValue, prot)) {
if (use_deleter) {
R_RegisterCFinalizerEx(data_, r_deleter, static_cast<r_bool>(finalize_on_exit));
}
Expand All @@ -66,9 +67,19 @@ class external_pointer {
data_ = safe[Rf_shallow_duplicate](rhs.data_);
}

external_pointer(external_pointer&& rhs) { reset(rhs.release()); }
external_pointer(external_pointer&& rhs) {
data_ = rhs.data_;
rhs.data_ = R_NilValue;
}

external_pointer& operator=(external_pointer&& rhs) noexcept { reset(rhs.release()); }
external_pointer& operator=(external_pointer&& rhs) noexcept {
// This works even if `this == &rhs` because `data_` (a `cpp11::sexp`) handles
// the underlying resource.
data_ = rhs.data_;
// Order matters: first assign, then clear the RHS.
rhs.data_ = R_NilValue;
return *this;
}

external_pointer& operator=(std::nullptr_t) noexcept { reset(); };

Expand Down
Loading
Loading