Skip to content

Add info to Rcpp plugins to support TBB when creating packages#140

Open
paciorek wants to merge 2 commits into
mainfrom
tbb-pkging
Open

Add info to Rcpp plugins to support TBB when creating packages#140
paciorek wants to merge 2 commits into
mainfrom
tbb-pkging

Conversation

@paciorek
Copy link
Copy Markdown
Contributor

This addresses issue #127.

I think this is fine to merge in, but we should discuss the comment in the issue about $env seemingly not affecting how a generated package is built/installed to see if there is more work to do around the Rcpp plugins.

when creating packages (issue #127)
@paciorek
Copy link
Copy Markdown
Contributor Author

@perrydv and I discussed this.

This PR needs to be revisited in light of the Rcpp plugin probably not being used / easy to use.

Instead, handle in cppDefs by conditionally including the include director if TBB used. We can know this by putting info in auxEnv when we detect use of TBB.

in cppDef processing, not via Rcpp plugin.
@paciorek
Copy link
Copy Markdown
Contributor Author

@perrydv I have this working by inserting a conditional into cpp_nClassBaseClass$process_Compiler() that checks the auxEnv of the contained NFcompilers.

However, if I compile two nClasses together, one that uses TBB and one that doesn't, the preamble for the .h files for both classes have #define NCOMPILER_USES_TBB. That's because in cppDefs_2_RcppPacket, we have Hpreamble <- allCppDefs |> lapply(\(x) x$getHpreamble()) |> prep(), and the prep does unique(unlist), so everything gets put together into a single preamble.

Thoughts on whether this is ok? Presumably so since you set it up so that we don't have different preambles for different code files...

@paciorek
Copy link
Copy Markdown
Contributor Author

Also if you are looking at the code changes, see what you think about the organization of where I inserted the addition to Hpreamble.

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.

1 participant