build the bundled onetbb on windows when rtools has no onetbb (#269)

* link downstream packages against the tbb stub on windows

tbbLdFlags() returned only '-L<libs> -lRcppParallel' on Windows, leaving
downstream packages to resolve TBB symbols against whichever objects
happened to be pulled out of the static TBB when RcppParallel.dll was
linked. That export surface is incidental rather than declared, and
packages taking all of their PKG_LIBS from RcppParallelLibs() (e.g. rstan)
have no other way to ask for TBB.

Offer the stub library as well, after '-lRcppParallel' so the package's own
exports still take precedence and no dependency on the stub is recorded
unless something actually needs it. RcppParallel 5.1.11 and earlier appended
'-ltbb -ltbbmalloc' here for the same reason.

* build the bundled onetbb on windows when rtools has no onetbb

Rtools42 ships Intel TBB 2017, and configure adopted it purely on the
presence of a libtbb* archive. Downstream packages then compiled against
those headers, which is not viable: StanHeaders uses
tbb::this_task_arena::isolate unconditionally, TBB 2017 still gates it
behind TBB_PREVIEW_TASK_ISOLATION, and its library doesn't export
isolate_within_arena, so forcing the declaration into view wouldn't link
either. rstan 2.32.7 consequently stopped building on R 4.2 / Windows.

Only adopt the Rtools TBB when it is oneTBB, and otherwise build the
bundled copy from sources, as we already do elsewhere. Rtools42 provides
cmake 3.24.1 in the same directory as gcc, so this needs no new tooling.
It is built as a static library so that RcppParallel.dll takes exactly the
shape it does with an Rtools oneTBB -- oneTBB marks its entry points
__declspec(dllexport) on mingw, so the whole-archive re-export of tbbmalloc
carries over unchanged.

Toolchains that can build neither (e.g. Rtools40, which has no TBB and no
cmake) still fall back to tinythread: a missing or too-old cmake is only an
error off Windows, where the bundled build is the sole option.

Also add CI covering this. R CMD check gains an R 4.2 Windows job, and a new
'downstream' job compiles and loads a translation unit against the flags
RcppParallel advertises, exercising both this_task_arena::isolate and
task_scheduler_observer. R CMD check alone would not have caught the rstan
breakage: RcppParallel installed perfectly well throughout.

* add news entries for the windows tbb changes

* run the downstream ci job under bash

The default shell on Windows runners is PowerShell, where 'r' is an alias
for Invoke-History; 'R CMD INSTALL' was parsed as 'Invoke-History CMD
INSTALL' and failed before the job did anything useful.

* patch onetbb's assembler probe for native windows builds

oneTBB probes the GNU assembler version by compiling /dev/null, which does
not exist when a native Windows cmake drives mingw gcc. The compiler fails
with 'no input files', the version regex doesn't match, and math() then
chokes on the error text -- taking the whole configure down:

  CMake Error at cmake/compilers/GNU.cmake:65 (math):
    math cannot parse the expression: "g++.exe: error: /dev/null: No such
    file or directory ... * 1000 + ..."

MXE doesn't hit this because it cross-builds oneTBB from a POSIX host.

Use NUL on Windows, and treat an unparseable probe as an assembler too old
for waitpkg rather than a fatal error -- it only guards an optimization, and
Rtools42's GCC 10.4 is below the GCC 11 floor for -mwaitpkg regardless.

* don't pass -fstack-clash-protection to mingw when building onetbb

GCC 10.4 aborts compiling oneTBB's task_dispatcher.h with the flag on:

  internal compiler error: in seh_emit_stackalloc, at config/i386/winnt.c:1056
    370 |                 do_throw_noexcept([] { throw; });

a known GCC defect on mingw's SEH targets. It is a hardening flag rather
than a correctness one, so leave it off there.

Also fold the earlier /dev/null fix into a single patches/mingw_gnu_cmake.diff,
as both make oneTBB's GNU.cmake usable for a native mingw build.

* don't redeclare the legacy observer classes in the tbb stub

The bundled oneTBB carries a local 'provided for backwards compatibility'
block declaring tbb::internal::task_scheduler_observer_v3 and
tbb::interface6::task_scheduler_observer (added in c276b03; not upstream).
tbb-compat.cpp declares them too, which was fine while the stub was only
ever built against the pristine headers Rtools ships -- but building the
bundled oneTBB on Windows makes the two collide:

  tbb-compat.cpp:25:7: error: redefinition of
    'class tbb::internal::task_scheduler_observer_v3'

Have the bundled header advertise what it provides, and skip the stub's own
declarations when it does. Either way the stub still exports the out-of-line
observe(), which is the whole reason it exists.

Also spell that definition __TBB_EXPORTED_METHOD, matching its declaration
here and in the old TBB ABI; the two attributes are both no-ops on the
Windows targets we build for, so this is consistency rather than a fix.

* keep the legacy observe definition out of the bundled tbb on windows

The bundled oneTBB carries a local (non-upstream) definition of
tbb::internal::task_scheduler_observer_v3::observe in observer_proxy.cpp,
alongside the declarations in task_scheduler_observer.h. Once the bundled
copy is actually built on Windows, libtbb12.a and tbb-compat.o both define
it and the stub fails to link:

  libtbb12.a(observer_proxy.cpp.obj): multiple definition of
    'tbb::internal::task_scheduler_observer_v3::observe(bool)'

The stub has to own that definition on Windows, because R CMD SHLIB builds
the DLL's export list from the objects it compiles -- a definition sitting
in the archive would never make it into tmp.def. So leave it out of the
library there; other platforms keep it, where it serves binaries built
against RcppParallel 5.x.

Also stop trusting R CMD SHLIB's exit code: it reported success through the
link failure above, so the install completed and shipped a package with no
tbb.dll -- precisely the failure mode that packages linking '-ltbb' only
hit at load time. Check for the artifact, and for the copy, instead.

* capture the legacy tbb abi modifications as a patch

The bundled oneTBB carries local declarations and a definition of the old
ABI's task_scheduler_observer, added in c276b03 so that binaries built
against RcppParallel 5.x keep working. Neither had a file in patches/, and
tools/tbb/update-tbb.R wipes src/tbb wholesale -- so the next oneTBB bump
would have dropped them silently, breaking the Windows tbb.dll stub in a
way that is hard to trace back.

Verified by applying the patch to a pristine oneTBB 2022.0.0 tree and
confirming the result matches what we ship.

* add a manually-triggered rstan build workflow

The 'downstream' job stands in for rstan in the normal CI run: it compiles
a translation unit using the two TBB APIs StanHeaders needs, in seconds
rather than the best part of an hour. This runs the real thing, on demand,
for when that's worth waiting for -- before a release, or after touching
the Windows TBB setup.

workflow_dispatch inputs choose the R version (4.2 by default, the one whose
Rtools has no oneTBB), the runner, and whether to additionally compile and
fit a tiny Stan model. The last exercises the TBB runtime at run time, not
just the headers and link line, so it's opt-in.

* correct rstan's dependency list and pin the package under test

Drop V8 and withr: rstan 2.32 replaced V8 with QuickJSR, and V8 failing to
install on an old toolchain would have failed the job for reasons that have
nothing to do with RcppParallel. The list now mirrors rstan's Imports plus
LinkingTo.

Install StanHeaders and rstan with dependencies = FALSE, and assert
afterwards that RcppParallel is still the .9000 build from this branch --
otherwise dependency resolution could quietly replace it with CRAN's, and
the run would report on the released package instead of this one.

* capture the functional oneTBB modifications as patches

Comparing src/tbb against a pristine oneTBB 2022.0.0 turns up 41 modified
files. Most are workarounds for over-zealous CRAN checks -- commented-out
'#pragma warning' / '#pragma GCC diagnostic' directives and the like -- and
are not worth carrying. Seven are not:

- src/tbb/def/{lin32,lin64,mac64}-tbb.def add
  tbb::internal::task_scheduler_observer_v3::observe to the export list,
  under a comment reading 'Needed by rstan', along with std:: exception
  typeinfo and destructors for backwards compatibility. These are the
  Linux/macOS counterpart of the Windows tbb.dll stub, so losing them would
  break rstan on exactly the platforms where it currently works.

- The four CMakeLists.txt files drop VERSION / SOVERSION, so the build emits
  unversioned libraries. install.libs.R's versionBundledTbbLibraries() then
  re-creates the 'libtbb.so.2' layout (#260); the two have to stay in step.

Both patches verified by applying them to a pristine tree and confirming
every touched file matches what we ship.

* note the tbb stub install failure fix in news

Author: kevinushey

.github/scripts/rstan-fit-check.R

@@ -0,0 +1,40 @@
+# Compile and fit a trivial Stan model. Building rstan proves the headers and
+# link line are right; actually running a model additionally exercises the TBB
+# runtime that RcppParallel loaded -- including the reduce_sum / task isolation
+# path that the old Rtools42 TBB could not support.
+
+library(rstan)
+
+code <- "
+data {
+  int<lower=0> N;
+  vector[N] y;
+}
+parameters {
+  real mu;
+}
+model {
+  y ~ normal(mu, 1);
+}
+"
+
+set.seed(42)
+data <- list(N = 20L, y = rnorm(20, mean = 3))
+
+fit <- stan(
+  model_code = code,
+  data       = data,
+  chains     = 1L,
+  iter       = 200L,
+  refresh    = 0L
+)
+
+estimate <- mean(rstan::extract(fit, "mu")[["mu"]])
+writeLines(sprintf("posterior mean of mu: %.3f (data mean %.3f)", estimate, mean(data$y)))
+
+# a wildly wrong answer would mean the model ran but the sampler is broken;
+# with 20 observations and unit scale this is a very loose bound
+if (!is.finite(estimate) || abs(estimate - mean(data$y)) > 1)
+   stop("posterior mean is implausible; the fit did not work correctly")
+
+writeLines("** rstan fit check passed")

.github/scripts/tbb-downstream-check.R

@@ -0,0 +1,106 @@
+# Build and load a translation unit exercising the parts of the TBB API that
+# downstream packages depend on, using only the flags RcppParallel advertises.
+#
+# StanHeaders -- and so rstan -- needs both of the pieces used here:
+# tbb::this_task_arena::isolate, to keep its thread-local AD tape from being
+# modified by a task from another arena, and tbb::task_scheduler_observer, to
+# install that tape on each worker. Rtools42's TBB 2017 provides neither in a
+# usable form, which is what broke rstan on R 4.2 (isolate is gated behind
+# TBB_PREVIEW_TASK_ISOLATION, and isolate_within_arena isn't in the library at
+# all), so this stands in for a full rstan build.
+
+code <- '
+#include <tbb/blocked_range.h>
+#include <tbb/parallel_for.h>
+#include <tbb/task_arena.h>
+#include <tbb/task_scheduler_observer.h>
+
+#include <atomic>
+#include <cstddef>
+
+// R.h remaps names like \'length\' onto Rf_ equivalents by default, which
+// collides with the standard library; keep it last, and unremapped
+#define R_NO_REMAP
+#include <R.h>
+#include <Rinternals.h>
+
+namespace {
+
+// mirrors StanHeaders\' ad_tape_observer, which is what pulls the
+// task_scheduler_observer entry points into the link
+struct observer : public tbb::task_scheduler_observer {
+   observer() : tbb::task_scheduler_observer() { observe(true); }
+   void on_scheduler_entry(bool) override {}
+   void on_scheduler_exit(bool) override {}
+};
+
+} // end anonymous namespace
+
+extern "C" SEXP tbb_downstream_check(void) {
+
+   observer obs;
+   std::atomic<int> total(0);
+
+   // mirrors StanHeaders\' use of task isolation in reduce_sum / map_rect
+   tbb::this_task_arena::isolate([&] {
+      tbb::parallel_for(
+         tbb::blocked_range<std::size_t>(0, 1024),
+         [&](const tbb::blocked_range<std::size_t>& range) {
+            total += static_cast<int>(range.end() - range.begin());
+         }
+      );
+   });
+
+   return Rf_ScalarInteger(total.load());
+
+}
+'
+
+# report how RcppParallel is configured, so a failure here can be read
+# against the provenance logged during installation
+writeLines(c(
+   sprintf("TBB_ENABLED : %s", RcppParallel:::TBB_ENABLED),
+   sprintf("TBB_LIB     : '%s'", RcppParallel:::TBB_LIB),
+   sprintf("TBB_INC     : '%s'", RcppParallel:::TBB_INC),
+   sprintf("CxxFlags()  : %s", RcppParallel:::tbbCxxFlags()),
+   sprintf("LdFlags()   : %s", RcppParallel:::tbbLdFlags())
+))
+
+if (!RcppParallel:::TBB_ENABLED)
+   stop("RcppParallel was installed without a tbb backend")
+
+# mirror rstan's src/Makevars.win, which takes all of its TBB configuration
+# from RcppParallel: its compiler flags via CxxFlags(), its linker flags via
+# RcppParallelLibs(), and the include path itself via 'LinkingTo'
+makevars <- c(
+   "CXX_STD = CXX17",
+   sprintf("PKG_CPPFLAGS = -I\"%s\"", system.file("include", package = "RcppParallel")),
+   "PKG_CPPFLAGS += $(shell \"${R_HOME}/bin/Rscript\" -e \"RcppParallel::CxxFlags()\" | tail -n 1)",
+   "PKG_LIBS += $(shell \"${R_HOME}/bin${R_ARCH_BIN}/Rscript\" -e \"RcppParallel::RcppParallelLibs()\" | tail -n 1)"
+)
+
+dir <- tempfile("tbb-downstream-")
+dir.create(dir, recursive = TRUE)
+owd <- setwd(dir)
+on.exit(setwd(owd), add = TRUE)
+
+writeLines(code, "check.cpp")
+
+# deliberately not named 'Makevars': R CMD SHLIB reads a Makevars in the
+# working directory as well as R_MAKEVARS_USER, and would apply both. going
+# through R_MAKEVARS_USER also keeps any personal ~/.R/Makevars out of the way
+writeLines(makevars, "makevars-downstream")
+Sys.setenv(R_MAKEVARS_USER = file.path(dir, "makevars-downstream"))
+status <- system(paste(shQuote(file.path(R.home("bin"), "R")), "CMD SHLIB check.cpp"))
+if (status != 0L)
+   stop("downstream translation unit failed to build")
+
+# loading also proves any load-time dependency on the tbb stub resolves
+dll <- dyn.load(paste0("check", .Platform$dynlib.ext))
+on.exit(dyn.unload(dll[["path"]]), add = TRUE)
+
+result <- .Call("tbb_downstream_check")
+if (!identical(result, 1024L))
+   stop("downstream check returned ", result, "; expected 1024")
+
+writeLines("** downstream TBB check passed")

.github/workflows/R-CMD-check.yaml

@@ -23,6 +23,9 @@ jobs:
           - {os: ubuntu-24.04-arm, r: 'release'}
           - {os: windows-latest,   r: 'release'}
           - {os: windows-11-arm,   r: 'release'}
+          # Rtools42 provides only a pre-oneTBB copy of TBB, so this is the
+          # one configuration that builds the bundled oneTBB on Windows
+          - {os: windows-latest,   r: '4.2'}
 
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
@@ -32,6 +35,7 @@ jobs:
 
       - uses: r-lib/actions/setup-r@v2
         with:
+          r-version: ${{ matrix.config.r }}
           use-public-rspm: true
 
       - uses: r-lib/actions/setup-r-dependencies@v2
@@ -40,3 +44,41 @@ jobs:
           needs: check
 
       - uses: r-lib/actions/check-r-package@v2
+
+  # Verify that a package can still compile and link against the TBB API that
+  # RcppParallel advertises. This is the contract rstan (via StanHeaders)
+  # depends on, and it is not covered by R CMD check: RcppParallel can install
+  # perfectly well while leaving downstream packages unable to build.
+  downstream:
+    runs-on: windows-latest
+
+    name: downstream (${{ matrix.r }})
+
+    strategy:
+      fail-fast: false
+      matrix:
+        r: ['4.2', 'release']
+
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      VERBOSE: 1
+
+    # the default shell on Windows runners is PowerShell, which aliases 'r'
+    # to Invoke-History and so mangles 'R CMD INSTALL'
+    defaults:
+      run:
+        shell: bash
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          r-version: ${{ matrix.r }}
+          use-public-rspm: true
+
+      - name: install RcppParallel
+        run: R CMD INSTALL --preclean .
+
+      - name: build against RcppParallel
+        run: Rscript .github/scripts/tbb-downstream-check.R

.github/workflows/rstan.yaml

@@ -0,0 +1,96 @@
+# Build rstan from source against this branch's RcppParallel.
+#
+# The 'downstream' job in R-CMD-check.yaml is a stand-in for this: it compiles a
+# small translation unit using the two TBB APIs StanHeaders needs. That catches
+# the breakage this workflow exists to rule out, in seconds rather than the best
+# part of an hour -- so this is deliberately manual, for when the real thing is
+# worth waiting for (before a release, or after touching the Windows TBB setup).
+on:
+  workflow_dispatch:
+    inputs:
+      r-version:
+        description: "R version (4.2 uses Rtools42, which has no oneTBB)"
+        required: true
+        default: "4.2"
+      os:
+        description: "Runner to build on"
+        required: true
+        default: "windows-latest"
+        type: choice
+        options:
+          - windows-latest
+          - macOS-latest
+          - ubuntu-latest
+      fit-model:
+        description: "Also compile and fit a tiny Stan model (slow)"
+        required: false
+        default: false
+        type: boolean
+
+name: rstan
+
+jobs:
+  rstan:
+    runs-on: ${{ inputs.os }}
+
+    name: rstan (${{ inputs.os }}, R ${{ inputs.r-version }})
+
+    # building rstan from source is slow, and slower still on Windows
+    timeout-minutes: 120
+
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      VERBOSE: 1
+      # rstan's own guidance for building it from source
+      MAKEFLAGS: -j2
+
+    defaults:
+      run:
+        shell: bash
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          r-version: ${{ inputs.r-version }}
+          use-public-rspm: true
+
+      - name: install RcppParallel from this branch
+        run: R CMD INSTALL --preclean .
+
+      - name: report how RcppParallel resolved TBB
+        run: |
+          Rscript -e 'writeLines(c(
+            paste("TBB_ENABLED", RcppParallel:::TBB_ENABLED),
+            paste("TBB_LIB   ", RcppParallel:::TBB_LIB),
+            paste("CxxFlags  ", RcppParallel:::tbbCxxFlags()),
+            paste("LdFlags   ", RcppParallel:::tbbLdFlags())
+          ))'
+
+      # binaries are fine for everything except rstan and StanHeaders, which are
+      # the packages that actually consume RcppParallel's flags. this is
+      # rstan's Imports plus its LinkingTo, minus RcppParallel (installed above)
+      # and StanHeaders (built from source below)
+      - name: install rstan's dependencies
+        run: |
+          Rscript -e 'install.packages(c("Rcpp","RcppEigen","BH","inline","gridExtra","loo","pkgbuild","QuickJSR","ggplot2"))'
+
+      # dependencies = FALSE so that resolving StanHeaders/rstan can't quietly
+      # pull CRAN's RcppParallel over the one we just built -- that would turn
+      # this into a test of the released package instead
+      - name: install StanHeaders and rstan from source
+        run: |
+          Rscript -e 'install.packages(c("StanHeaders","rstan"), type = "source", dependencies = FALSE, INSTALL_opts = "--no-multiarch")'
+
+      - name: check RcppParallel is still the one we built
+        run: |
+          Rscript -e 'v <- as.character(packageVersion("RcppParallel")); writeLines(paste("RcppParallel", v)); if (!grepl("[.]9000$", v)) stop("RcppParallel was replaced by a released build; the rstan build did not test this branch")'
+
+      - name: load rstan
+        run: |
+          Rscript -e 'library(rstan); print(rstan::stan_version())'
+
+      - name: compile and fit a tiny model
+        if: ${{ inputs.fit-model }}
+        run: Rscript .github/scripts/rstan-fit-check.R

NEWS.md

@@ -6,6 +6,25 @@
   'random_access_iterator' in namespace 'std'". This affected CRAN's macOS
   x86_64 machines, which pair Apple clang 14 with the macOS 11.3 SDK.
 
+* On Windows, RcppParallel now only uses the copy of TBB provided by Rtools
+  when that copy is oneTBB; otherwise, the bundled oneTBB is built from
+  sources, as it already is on other platforms. Rtools42 provides Intel TBB
+  2017, whose headers downstream packages cannot build against: StanHeaders
+  uses `tbb::this_task_arena::isolate`, which that release still gates behind
+  `TBB_PREVIEW_TASK_ISOLATION` and does not export from its library. As a
+  result, rstan could no longer be built on R 4.2 for Windows.
+
+* On Windows, `RcppParallel::RcppParallelLibs()` once again offers the TBB
+  stub library, so that packages taking all of their linker flags from it
+  (e.g. rstan) can resolve TBB symbols which `RcppParallel.dll` does not
+  itself re-export.
+
+* Fixed an issue on Windows where a failure to build the TBB stub library
+  could go unreported, since `R CMD SHLIB` has been seen to exit successfully
+  even when the link failed. Installation completed, but shipped a package
+  with no `tbb.dll` -- which packages linking `-ltbb` then discovered only
+  when they failed to load. Installation now fails instead.
+
 * Fixed an issue where building the bundled oneTBB could fail when `CXX`
   (or `CC`) was configured with a leading compiler launcher such as `ccache`
   (e.g. `CXX = "ccache g++"`). The launcher is now forwarded to cmake via

R/tbb.R

@@ -87,12 +87,25 @@ tbbLdFlags <- function() {
 
    # on Windows, we statically link to oneTBB
    if (is_windows()) {
-      
+
       libPath <- archSystemFile("libs")
 
       ldFlags <- sprintf("-L%s -lRcppParallel", asBuildPath(libPath))
+
+      # also offer the stub library. RcppParallel.dll re-exports whichever TBB
+      # objects happened to be pulled out of the static library when it was
+      # linked, which is an incidental export surface rather than a declared
+      # one; the stub exports the runtime wholesale, so anything missing from
+      # RcppParallel.dll can still be resolved. this comes last deliberately:
+      # the linker satisfies symbols in order, so RcppParallel's own exports
+      # still win, and no dependency on the stub is recorded unless something
+      # actually needs it
+      tbbPath <- archSystemFile("lib")
+      if (file.exists(file.path(tbbPath, "tbb.dll")))
+         ldFlags <- paste(ldFlags, sprintf("-L%s -ltbb", asBuildPath(tbbPath)))
+
       return(ldFlags)
-      
+
    }
 
    # shortcut if TBB_LIB defined