const-correct public C API

[castedo]

Following up on discussion with @molpopgen and @jeromekelleher regarding possibility of making tskit 1.0 const-correct (const-correct in the API being use of "const" in pointer parameter types declaration when API functions do not change the data pointed to).

I can't think of any downside for the callers of the API to have those parameter types get const'ed and I don't believe it will break any users (unless they are doing something unusual like keeping pointers to functions and assigning API functions to those function pointers).

In the public API, this is convenient to API users who use const often (e.g. @molpopgen and many C++ programmers). It is less annoying too. They don't have to cast away const-ness when calling the API.

However, for tskit implementors this can become a pain. This page has a nice description of the trade-off and uses the term
"const poisoning":
https://wiki.sei.cmu.edu/confluence/display/c/DCL00-C.+Const-qualify+immutable+objects

I can suggest initially only focusing on making the public API const-correct and not worrying about the broader use of const in the implementation of tskit. If there is a const issue introduced by this const type change, one can just blindly do something like this:

int public_api_func(double *values)
{
   ... code not "speaking in const" ...
}

goes to

int public_api_func(double const *_values)
{
    double *values = (double*)_values;
   ... same code not "speaking in const" ...
}

And if there is no const issue, then no need to do this.

In a sense, this is putting the burden of casting away const-ness on the implementors of tskit rather than users who call it. Which seems to be the right place to put that burden.

[benjeffery]

Yep, I agree this is a good thing to do. I've caught issues before by "constifiying" some C++ code. I think it also helps the API become a bit more self-documenting.

[jeromekelleher]

OK, added to the C 1.0 milestone.

[molpopgen]

While I am in favor of this in general, it could be a big load of work that goes nowhere. In my experience, it is hard to constify a code base after the fact.

C allows 3 distinct uses of const in a function prototype. If the goal is to document to the user that their
input data are guaranteed to be unmodified by the function, only one of the 3 uses satisfies that goal and is also
compatible with a C++ compiler.

Personally, I prefer foo(const double * x) to foo(double * const x). Given that msprime uses the GSL, and the former convention is used there, it may be confusing to have different flavors of const in play for people new to looking at the code.
For the latter case, the user's input data are still mutable.

#include <stdlib.h>
#include <stdio.h>
#include <assert.h>

/* This is what we really mean most of 
 * the time--complete immutability.
 * However, I find it close to unreadable,
 * and it is not valid when run thru
 * a C++ compiler.
 */
void
foo(const double const *x)
{
}

/* The most common form in my experience.
 * Also what the GSL docs use.
 * Arguably better-aligned w/intuition
 * about what we want.
 */
void
foo2(const double *x)
{
    // Fails to compile:
    // x[0] = 1;
    double *y = calloc(10, sizeof(double));
    y[0] = 1;
    x = y; // Dang, that feels wrong, but the caller won't see it.
    fprintf(stdout, "%lf\n", x[0]);
    free(y);
}

/* 
 * This is the "the pointer is constant" version.
 * However, this isn't what the goal is most 
 * of the time.  Here, the input data
 * are still mutable!
 */
void
foo3(double *const x)
{
    x[0] = 1;
    // Fails to compile:
    // double * y = NULL;
    // x = y;
}

int
main(int argc, char **argv)
{
    double *x = calloc(10, sizeof(double));

    foo(x);
    foo2(x); /* the colling environment's x in unaffected */
    assert(x != NULL);
    assert(x[0] == 0);
    fprintf(stdout, "%lf\n", x[0]);
    foo3(x); 
    assert(x[0] == 1);
}

[castedo]

Thanks @molpopgen for the example. I've incorporated it with some corrections into a reference page for "CNP #11" ... pronounced "see-nip" :)

http://www.castedo.com/cnp/11/

This page is half-humour, half-serious and I hope 100% fun for computational biologists.

There are some interesting facts about the foo(const int * x) vs foo(int const * x) variation, especially the Italian interpretation.

Y'all have the best sense what typical msprime C library users might be most familiar with. I'm guessing it's the 'ancestral variant'. But the mutant variation is quite invasive and there seems to be some selection pressure for its spread. :)

If any of you would like a change or addition, just let me know. The source is currently at https://github.com/castedo/tskit-bud/tree/main/cnp

[castedo]

So far there is one vote for "English style" adjective-noun const double * and none for "Mediterranean style" noun-adjective double const *. This probably matches best what is most commonly understood by most users of the C API.

@benjeffery feel free to assign this to me.