Functions that CRAN disallows

szhorvat · 27 November 2020 16:07

@Gabor The igraph C sources have many parts disabled with #ifndef USING_R.

I assume this is motivated by CRAN not allowing functions that exit (abort()) or printing to the terminal (stdout, stderr).

These rules are of course reasonable not only for R but for all the high-level igraph interfaces. But having all this USING_R is inconvenient and error-prone.

Thus, my question is: What are the exact rules of CRAN? Do they really not allow these symbols to be even referenced in any of the object files? The public guidelines simply say not to call these functions, but don’t explicitly forbid referencing them. An example is igraph’s error handler: the R interface sets its own error handler anyway. Having to comment out all the default handlers is pointless and inconvenient. Who can we clarify the requirements with?

CC @vtraag @tamas since we’re been discussing this this morning.

Gabor · 27 November 2020 16:38

Why is this error prone? If you need to do compile time config, how else would you do it?

The package has to pass R CMD check with CRAN’s configs.

R CMD check checks the shared lib of the package and fails if it includes them. Why do you think I made an effort to remove them? It is particularly annoying to remove them from the flex/bison generated files.

I am not sure how that is relevant, that does not use abort() etc, but throws an R error.

Well, we don’t get to make the CRAN rules. If you want to update the package on CRAN, you need to follow the rules. Frankly, arguing with CRAN is pointless and a waste of time. They don’t have a way to exclude false positives from their checks, so we need to work around them.

szhorvat · 27 November 2020 18:02

It is error prone because it makes me touch R specific code when I am working on the C library.

This all came up while I was implementing fatal error handlers:

github.com/igraph/igraph

Implement fatal error handling

igraph:develop ← szhorvat:feature/igraph_fatal

opened 03:03PM - 21 Nov 20 UTC

szhorvat

+270 -53

This PR adds fatal error handling, as well as igraph-specific assertions. Whe…n a fatal error occurs, we don't do any cleanup, just bail immediately. This is necessary in some libraries where it would be difficult to integrate igraph's own error handler. This can also be used for assertions that we _do not_ want to be disabled by `NDEBUG`. Some asserts don't hurt performance, and the messages they print can be very useful to debug user problems. Also, when an assert macro is done properly, the compiler can be aware that is never returns, and [can optimize the code much better](https://stackoverflow.com/a/47183046/695132). Thus, the idea that assertions should simply be removed for better performance is flawed. A fatal error handler is expected to never return. Reasonable options are to `abort()` or to do a `longjmp`. The R interface would use the `error()` function provided by R, which simply returns to the R top level after printing a message. See the modification I made in the Cliquer sources—originally, R's `error()` was used. This is still a draft, awaiting feedback before I go further.

To make a long story short, I needed something that doesn’t return to make the compiler happy about the “noreturn” attribute. This would normally be an abort(). It would never get called if the rest of the code is correct, but in this situation the compiler had no way to determine that, so it gave a warning. Since we now use -Werror, it got inconvenient.

In the end, I used exit() from R.h, but didn’t have the chance to actually test this yet (updating the R interface to use the new cmake build system is going to take a while).

github.com/igraph/igraph

Implement fatal error handling

igraph:develop ← szhorvat:feature/igraph_fatal

opened 03:03PM - 21 Nov 20 UTC

szhorvat

+270 -53

This PR adds fatal error handling, as well as igraph-specific assertions. Whe…n a fatal error occurs, we don't do any cleanup, just bail immediately. This is necessary in some libraries where it would be difficult to integrate igraph's own error handler. This can also be used for assertions that we _do not_ want to be disabled by `NDEBUG`. Some asserts don't hurt performance, and the messages they print can be very useful to debug user problems. Also, when an assert macro is done properly, the compiler can be aware that is never returns, and [can optimize the code much better](https://stackoverflow.com/a/47183046/695132). Thus, the idea that assertions should simply be removed for better performance is flawed. A fatal error handler is expected to never return. Reasonable options are to `abort()` or to do a `longjmp`. The R interface would use the `error()` function provided by R, which simply returns to the R top level after printing a message. See the modification I made in the Cliquer sources—originally, R's `error()` was used. This is still a draft, awaiting feedback before I go further.

One could also use while (1) {} but that’s just nasty.

My point was that the R interface never makes a call to abort() even if functions that reference abort() are left in.

I guess there’s not much we can do, but I really do wonder how a lot of useful C packages even make it into R if the rules effectively require anyone who would want to create an R interface to them to do invasive patching …

Gabor · 27 November 2020 18:50

Well, R does not need any error handler, so all that can be in #ifdef USING_R. Alternatively you can refactor the code so that the (default) error handlers of the C library are defined in a separate file, which we’ll leave out from the R interface.

Another way would be to not include any R specific code in the C library, and have a patch that is applied for creating the C code of the R package. I find this a lot more error prone, though.

I don’t think there is exit() in R.h, and you should not call the exit() libc function.

Most packages don’t embed libraries but link to them, and these rules do not apply to the linked libraries.

szhorvat · 27 November 2020 19:26

There isn’t exit. I’m tired and I meant to type error. It is marked as noreturn , so it served the purpose.

I see that th elinks above are broken, here’s a good link:

github.com

igraph/igraph/blob/develop/src/igraph_error.c#L54


* is never actually called by it.
*
* Note that some of the other #ifndef USING_R's in this file are still needed
* to avoid references to fprintf and stderr.
*/
static IGRAPH_NORETURN void igraph_abort() {
#ifndef USING_R
   abort();
#else
   /* R's error() function is declared 'noreturn'. We use it here to satisfy the compiler that igraph_abort() does indeed not return. */
   error("igraph_abort() was called. This should never happen. Please report this as an igraph bug, along with steps to reproduce it.");
#endif
}
/***** Handling errors *****/
static IGRAPH_THREAD_LOCAL igraph_error_handler_t *igraph_i_error_handler = 0;
static IGRAPH_THREAD_LOCAL char igraph_i_errormsg_buffer[500];
static IGRAPH_THREAD_LOCAL char igraph_i_warningmsg_buffer[500];
static IGRAPH_THREAD_LOCAL char igraph_i_fatalmsg_buffer[500];

Topic		Replies	Views
R crashes due to an error in vertex.attr.comb and edge.attr.comb functions Usage C , R	2	517	29 July 2020
Erratic behaviour of make_empty_graph(NULL) Usage C , R	7	422	26 August 2020
On the design of igraph_eulerian_path Development C	1	568	13 September 2020
Error with igraph::layout Usage R	6	1497	17 July 2020
Do not use IGRAPH_NAN in the docs Development	0	337	1 March 2020

Functions that CRAN disallows

Related Topics