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

Needed update to src/Rcpp_init.cpp for new registration #651

Closed
eddelbuettel opened this issue Feb 26, 2017 · 6 comments
Closed

Needed update to src/Rcpp_init.cpp for new registration #651

eddelbuettel opened this issue Feb 26, 2017 · 6 comments

Comments

@eddelbuettel
Copy link
Member

When @kevinushey opened #636 he showed

* checking compiled code ... NOTE
File ‘anytime/libs/anytime.so’:
  Found no calls to: ‘R_registerRoutines’, ‘R_useDynamicSymbols’

It is good practice to use registered native symbols and to disable
symbol search.

See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual.

Re-running this with yesterday's R-devel on yesterday's Rcpp I got

* checking compiled code ... NOTE
File ‘Rcpp/libs/Rcpp.so’:
  Found no call to: ‘R_useDynamicSymbols’

It is good practice to register native routines and to disable symbol search.

See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual.
* checking sizes of PDF files under ‘inst/doc’ ... OK

Running the skeleton generator yesterday I got

R> tools::package_native_routine_registration_skeleton(".")
#include <R.h>
#include <Rinternals.h>
#include <stdlib.h> // for NULL
#include <R_ext/Rdynload.h>

/*
  The following symbols/expresssions for .NAME have been omitted

    Class__name
    Module__functions_arity
    Module__name
    Module__classes_info
    Module__complete
    CppClass__complete
    rcpp_error_recorder
    InternalFunction_invoke
    Module__get_function
    Module__get_class
    class__newInstance
    class__dummyInstance
    symbol
    Module__functions_names
    CppMethod__invoke_void
    CppMethod__invoke_notvoid
    CppMethod__invoke
    CppObject__finalize
    Class__has_default_constructor
    CppField__get
    CppField__set
    rcpp_capabilities
    as_character_externalptr

  Most likely possible values need to be added below.
*/

/* FIXME: 
   Check these declarations against the C/Fortran source code.
*/

/* .Call calls */
extern SEXP compileAttributes(SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP, SEXP);
extern SEXP sourceCppContext(SEXP, SEXP, SEXP, SEXP, SEXP);

static const R_CallMethodDef CallEntries[] = {
    {"compileAttributes", (DL_FUNC) &compileAttributes, 8},
    {"sourceCppContext",  (DL_FUNC) &sourceCppContext,  5},
    {NULL, NULL, 0}
};

void R_init_Rcpp(DllInfo *dll)
{
    R_registerRoutines(dll, NULL, CallEntries, NULL, NULL);
    R_useDynamicSymbols(dll, FALSE);
}
R> 

and I plan to fold that in later once the small PR for the Rcpp FAQ is through. Ok?

@kevinushey
Copy link
Contributor

I think we're going to need

R_useDynamicSymbols(dll, TRUE);

Yet as we discussed separately that wasn't needed on RcppArmadillo for some surprising reason but I think "in theory" any foreign method calls of the form .Call("method") should fail if that's off.

@kevinushey
Copy link
Contributor

It's also possible we could get away with just adding that line to our current init routine here:

Rcpp/src/Rcpp_init.cpp

Lines 128 to 136 in 8bf15c0

extern "C" void R_init_Rcpp(DllInfo* info) {
setCurrentScope(0);
registerFunctions();
init_Rcpp_cache(); // init the cache
init_Rcpp_routines(info); // init routines
}

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Feb 26, 2017

Good call. Seems to pan out.

edd@max:~/git$ rdcc.sh Rcpp_0.12.9.4.tar.gz                ## simple wrapper using my local R-devel
* using log directory ‘/home/edd/git/Rcpp.Rcheck’
* using R Under development (unstable) (2017-02-25 r72256)
* using platform: x86_64-pc-linux-gnu (64-bit)
* using session charset: UTF-8
* using option ‘--as-cran’
* checking for file ‘Rcpp/DESCRIPTION’ ... OK
* this is package ‘Rcpp’ version ‘0.12.9.4’
* checking CRAN incoming feasibility ... Note_to_CRAN_maintainers
Maintainer: ‘Dirk Eddelbuettel <edd@debian.org>’
* checking package namespace information ... OK
* checking package dependencies ... OK
* checking if this is a source package ... OK
* checking if there is a namespace ... OK
* checking for executable files ... OK
* checking for hidden files and directories ... OK
* checking for portable file names ... OK
* checking for sufficient/correct file permissions ... OK
* checking whether package ‘Rcpp’ can be installed ... OK
* checking installed package size ... NOTE
  installed size is 10.2Mb
  sub-directories of 1Mb or more:
    doc       1.7Mb
    include   6.3Mb
* checking package directory ... OK
* checking ‘build’ directory ... OK
* checking DESCRIPTION meta-information ... OK
* checking top-level files ... OK
* checking for left-over files ... OK
* checking index information ... OK
* checking package subdirectories ... OK
* checking R files for non-ASCII characters ... OK
* checking R files for syntax errors ... OK
* checking whether the package can be loaded ... OK
* checking whether the package can be loaded with stated dependencies ... OK
* checking whether the package can be unloaded cleanly ... OK
* checking whether the namespace can be loaded with stated dependencies ... OK
* checking whether the namespace can be unloaded cleanly ... OK
* checking loading without being on the library search path ... OK
* checking use of S3 registration ... OK
* checking dependencies in R code ... OK
* checking S3 generic/method consistency ... OK
* checking replacement functions ... OK
* checking foreign function calls ... NOTE
Registration problem:
  symbol ‘symbol’ in the local frame:
   .Call(symbol)
See chapter ‘System and foreign language interfaces’ in the ‘Writing R Extensions’ manual.
* checking R code for possible problems ... OK
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd line widths ... OK
* checking Rd cross-references ... OK
* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking line endings in C/C++/Fortran sources/headers ... OK
* checking line endings in Makefiles ... OK
* checking compilation flags in Makevars ... OK
* checking for GNU extensions in Makefiles ... OK
* checking for portable use of $(BLAS_LIBS) and $(LAPACK_LIBS) ... OK
* checking compiled code ... OK
* checking sizes of PDF files under ‘inst/doc’ ... OK
* checking installed files from ‘inst/doc’ ... OK
* checking files in ‘vignettes’ ... OK
* checking examples ... OK
* checking for unstated dependencies in ‘tests’ ... OK
* checking tests ...
  Running ‘doRUnit.R’ [188s/167s]
 OK
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE

Status: 2 NOTEs
See
  ‘/home/edd/git/Rcpp.Rcheck/00check.log’
for details.


edd@max:~/git$ 

The change is minimal, and besides cosmetics really just the addition you suggested.

I am still unsure over R_useDynamicSymbols(dll, TRUE); versus R_useDynamicSymbols(dll, FALSE); both seem to work.

@eddelbuettel
Copy link
Member Author

One possible solution to the puzzle, just triggered for an R CMD check on RInside (which is course not your standard package):

It is good practice to register native routines and to disable symbol search.

Also not that a grep for R_useDynamicSymbols in doc/manual/ for R-devel only shows FALSE used...

@eddelbuettel
Copy link
Member Author

I think we are good here. I ran a full set of regression just before the one-but-last weekend (logs here) and I think we should be generally ready for a new release.

@eddelbuettel
Copy link
Member Author

This should be good. It went into 0.12.10 after all.

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