Manage abort, assert, stderr, and stdout in C library for embedding it into R (and other similar environments)

[gregorgorjanc]

One niggle I am encountering with building R package for exposing tskit C API (https://github.com/HighlanderLab/tskitr) is this kind of warning when running devtools::check() (we have to iron out all errors and warnings before eventual submission to CRAN):

...
── R CMD check results ──────────────────────── tskitr 0.0.1 ────
Duration: 30.8s

❯ checking compiled code ... WARNING
  File 'tskitr/libs/tskitr.so':
    Found '___assert_rtn', possibly from 'assert' (C)
      Objects: 'tskit/haplotype_matching.o', 'tskit/kastore.o',
        'tskit/tables.o', 'tskit/trees.o'
    Found '___stderrp', possibly from 'stderr' (C)
      Objects: 'tskit/core.o', 'tskit/genotypes.o',
        'tskit/haplotype_matching.o', 'tskit/tables.o', 'tskit/trees.o'
    Found '___stdoutp', possibly from 'stdout' (C)
      Object: 'tskit/core.o'
    Found '_abort', possibly from 'abort' (C)
      Objects: 'tskit/core.o', 'tskit/genotypes.o',
        'tskit/haplotype_matching.o', 'tskit/tables.o', 'tskit/trees.o'
  
  Compiled code should not call entry points which might terminate R nor
  write to stdout/stderr instead of to the console, nor use Fortran I/O
  nor system RNGs nor [v]sprintf.
  
  See 'Writing portable packages' in the 'Writing R Extensions' manual.

0 errors ✔ | 1 warning ✖ | 0 notes ✔
...

To replicate this warning follow https://github.com/HighlanderLab/tskitr/tree/main?tab=readme-ov-file#development. I have gathered some docs on this matter for myself at HighlanderLab/tskitr#5.

A standard way of dealing with this issue in R packages is to avoid crashing R session because the underlying process crashed and communicating abort/errors/warnings/messages to R session from the underlying process. I believe this is done using something like #ifdef R_BUILD (to be revised) and using Rcpp::stop for stderr (possibly also for assert though sometimes I see that in R packages these are removed from low-level code at compile time using -DNDEBUG - not sure if we should error though?, maybe using Rcpp::warning instead?) and similarly for stderr and stdout.

The above describes R way of doing things, but I wonder if we should be thinking of this more generally.

Any thoughts on this?

[jeromekelleher]

I think most of these will be coming from the tsk_bug_assert macro which you could either undefine at compile time (probably?) or we could add some flags to conditionally compile it.

Ideally, we would redefine the macro in the tskit-R c++ code, which could be as simple as

#define tsk_bug_assert(condition)                                                       \
    do {                                                                                \
        if (!(condition)) {                                                             \
            correct_way_to_raise_error_message("Bug detected in %s at line %d. %s\n", __FILE__, __LINE__,  \
                TSK_BUG_ASSERT_MESSAGE);                                                \
             Rcpp::stop()...                                                             \
        }                                                                               \
    } while (0)

Otherwise, there may be a few stray printfs, which I'm very happy to add conditional compiles for.

[jeromekelleher]

For the assert calls, I think compiling with -DNDEBUG is perfectly acceptable (and indeed the standard practise)

[gregorgorjanc]

Thanks for this. I have now used PKG_CFLAGS = -DNDEBUG to remove the warning about assert calls (HighlanderLab/tskitr@92d39c6). I will bite into redefining the tsk_bug_assert macro next and report back.

[gregorgorjanc]

I now also managed to address abort (redefining tsk_bug_assert HighlanderLab/tskitr@b2a29f1, I later also found kas_bug_assert() HighlanderLab/tskitr@00870d9) and stderr (redefining tsk_trace_error() HighlanderLab/tskitr@00870d9). All downstream, so no need to do anything in tskit!

I am now down to this feedback from R CMD check:

...
✔  checking pragmas in C/C++ headers and code ...
✔  checking compilation flags used ...
N  checking compiled code ...
   File ‘tskitr/libs/tskitr.so’:
     Found ‘___stdoutp’, possibly from ‘stdout’ (C)
       Object: ‘tskit/core.o’
   
   Compiled code should not call entry points which might terminate R nor
   write to stdout/stderr instead of to the console, nor use Fortran I/O
   nor system RNGs nor [v]sprintf.
   
   See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual.
✔  checking examples (4.2s)
...

Searching for stdout I only find it used in core.h

FILE *
tsk_get_debug_stream(void)
{
    if (_tsk_debug_stream == NULL) {
        _tsk_debug_stream = stdout;
    }
    return _tsk_debug_stream;
}

This one is more challenging for me ... If I understand this correctly, it returns a FILE stream (?) and this is how it's used downstream, say in tsk_matvec_calculator_print_state(const tsk_matvec_calculator_t *self, FILE *out). I don't know how to go around here.

Any suggestions?

Thanks!

[jeromekelleher]

This is a trickier one and will probably need a small change to tskit C to accomodate. I've made a draft proposal in #3364 which might work. Is there a "stdout"-like file stream available in R that C code can write to?

If not then we can probably just define TSK_DEFAULT_DEBUG_STREAM to NULL, but we would need to make sure that the TSK_DEBUG flag cannot be passed to any of the C functions (which should be straightforward I think, but it depends on how much control you give the R code over the low-level parameters passed to C).

We could also #define away all the calls to the x_print_state functions, but I think that would lead to different compiler warnings.

[gregorgorjanc]

I hunted this one down in the R extensions manual - https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Printing-1, which says

The most useful function for printing from a C routine compiled into R is Rprintf.
This is used in exactly the same way as printf, but is guaranteed to write to R’s output
(which might be a GUI console rather than a file, and can be re-directed by sink).
It is wise to write complete lines (including the "\n") before returning to R. It is defined in R_ext/Print.h.

The function REprintf is similar but writes on the error stream (stderr)
which may or may not be different from the standard output stream.

Functions Rvprintf and REvprintf are analogues using the vprintf interface.
Because that is a C99[168](https://cran.r-project.org/doc/manuals/r-release/R-exts.html#FOOT168) interface,
they are only defined by R_ext/Print.h in C++ code if the macro R_USE_C99_IN_CXX
is defined before it is included or (as from R 4.0.0) a C++11 compiler is used.

Another circumstance when it may be important to use these functions
is when using parallel computation on a cluster of computational nodes,
as their output will be re-directed/logged appropriately.

So for R side we should send the output to these functions so it can be captured by R. The functions are Rprintf() and REprintf(). There is also Rf_error() and Rf_warning() but we are only interested in Rprintf() here.

Would this work (I am on thin ice here): If you #define FILE *tsk_get_debug_stream() I could redefine it to call Rprintf() like I did with tsk_bug_assert()?

[jeromekelleher]

It's not worth the bother. These are just debug messages for C developers, and are not enabled in the Python interface. We'll just set the stream to NULL as above, and just make sure the debug flag can't be passed.