Type-safe typecasts

Type-safe typecasts

[Marc Nieper-Wißkirchen]

[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]

Hi,

I have been wondering whether it makes sense to add a small utility trying
to make typecasts in C as type-safe as possible.

The problem is that typecasts are sometimes unavoidable.  For an example,
let's take a look at the following snippet using Gnulib's xlist module:

struct foo
{
  int bar;
};
gl_list_t list = gl_list_create_empty (GL_ARRAY_LIST, NULL, NULL, NULL,
false);
struct foo foo = { .bar = 1 };
gl_list_add_last (list, &foo);
gl_list_iterator_t i = gl_list_iterator (list);
struct foo *elt;
while (gl_list_iterator_next (&i, (const void **) &elt, NULL))
  ++elt->bar;
gl_list_iterator_free (&i);

Here, the typecast '(const void **)' is needed as 'gl_list_iterator_next'
expects an argument of that type and because 'struct foo **elt' is not
implicitly convertible to 'const void **' (in fact, even not to 'void **').

The problem with this typecast is that the compiler wouldn't have
complained if I had forgotten to write the address operator '&' before
'elt'.

So what I have in mind are macros that do a type conversion from A to B and
that signal an error on modern compilers if the input is not of type A. For
this, the C11 construct _Generic can be used.

The macros could look like

CAST (<from-type>, <to-type>, <expr>)

CVR_CAST (<type>, expr) /* removes cvr qualifier */

ADDRESS_CAST (<from-type>, <to-type>, expr)

expanding, respectively, into

_Generic ((<expr>), (<from-type>): ((<to-type>) (<expr>)))

_Generic ((<expr>), (<type>): ((<type>) (<expr>)),
                    (const <type>): ((<type> (<expr>)),
                    (const volatile <type>): ((<type>) (<expr>)),
                    ...)

_Generic ((<expr>), (<from-type>): (((<to-type>) *) &(<expr>)))

The names and the exact semantics and number of macros are, of course,
debatable.

Marc

[-- Attachment #2: Type: text/html, Size: 5111 bytes --]

Re: Type-safe typecasts

[Paul Eggert]

On 4/6/21 12:18 AM, Marc Nieper-Wißkirchen wrote:
> So what I have in mind are macros that do a type conversion from A to B and
> that signal an error on modern compilers if the input is not of type A. For
> this, the C11 construct _Generic can be used.

Not sure it's worth the aggravation. Most of the time, the type you're 
converting to void * is obvious from context. (However, I guess I 
wouldn't object in code that I don't have to look at myself. :-)

By the way, the snippet you gave is not portable C code, as it assumes 
that 'void *' and 'struct foo *' have the same machine representation. 
This is not necessarily true on (admittedly now-rare) machines that have 
different flavors of pointers. I suspect the main problem here is either 
in the calling code, or in the API for gl_list_iterator_next: if it 
returned a possibly-null value of type 'const void *' instead of storing 
the result through a 'const void **' pointer, the calling code wouldn't 
have gotten into this portability mess.

Re: Type-safe typecasts

[Marc Nieper-Wißkirchen]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

Hi Paul,

thanks!

By the way, the snippet you gave is not portable C code, as it assumes
> that 'void *' and 'struct foo *' have the same machine representation.
> This is not necessarily true on (admittedly now-rare) machines that have
> different flavors of pointers. I suspect the main problem here is either
> in the calling code, or in the API for gl_list_iterator_next: if it
> returned a possibly-null value of type 'const void *' instead of storing
> the result through a 'const void **' pointer, the calling code wouldn't
> have gotten into this portability mess.
>

gl_list_iterator_next has to return two things: An element (represented by
a const void *) and a boolean value. As elements may be NULL, the boolean
value is really needed. Of course, one could switch the actual return
parameter with the one returned through a pointer but then it wouldn't look
as nice in while or for-loops.

So to make my original code portable C, I would have to code

...
const void *e;
while (gl_list_iterator_next (&i, &e, NULL))
  {
    struct foo *foo = (void *) e;
    ++foo->bar;
  }
...

The const typecast is, unfortunately, still needed to silence compiler
warnings as the Gnulib list API suffers a bit from const-poisoning when the
actual elements are pointers actually non-const objects.

Marc

[-- Attachment #2: Type: text/html, Size: 2822 bytes --]

Re: Type-safe typecasts

[Paul Eggert]

On 4/6/21 12:13 PM, Marc Nieper-Wißkirchen wrote:
> gl_list_iterator_next has to return two things: An element (represented by
> a const void *) and a boolean value. As elements may be NULL

Ah, OK, then that's the problem. The API shouldn't allow null elements. 
They're not that useful anyway. If they really are needed for some 
lists, I suppose one could have a more-complicated API to return them 
(by setting a bool elsewhere); but usually they aren't.

> So to make my original code portable C, I would have to code
> 
> ...
> const void *e;
> while (gl_list_iterator_next (&i, &e, NULL))
>   {
>     struct foo *foo = (void *) e;
>     ++foo->bar;
>   }
> ...
> 
> The const typecast is, unfortunately, still needed to silence compiler
> warnings

Yes, that would be portable. But that cast indicates another problem 
with the API. It should return void *, not void const * (think of strchr 
as the model here). The API should discourage type-casts, since they're 
more dangerous than the alternatives.

Re: Type-safe typecasts

[Ben Pfaff]

On Tue, Apr 6, 2021 at 12:18 AM Marc Nieper-Wißkirchen
<marc.nieper+gnu@gmail.com> wrote:
> I have been wondering whether it makes sense to add a small utility trying to make typecasts in C as type-safe as possible.

I've found a few macros for casts useful over years. PSPP uses
CONST_CAST and UP_CAST from the file below quite a bit:
https://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h

Re: Type-safe typecasts

[Marc Nieper-Wißkirchen]

[-- Attachment #1: Type: text/plain, Size: 1954 bytes --]

CCing Bruno because of his involvement with the Gnulib list modules.

Disallowing NULL list elements could break existing code that actually uses
them but returning elements with type void * instead of const void * would
be much less incompatible. Code can trivially be ported to such an updated
interface (for example, in my snippet above, I would have to replace 'const
void *e' with 'void *e', All in all, it would make working with the code a
lot easier.

For lists where there are no non-null list elements, one could add

void *gl_list_iterator_next_element_or_null (gl_list_iterator_t iter,
gl_list_node_t *node_ptr)
{
  void *e;
  return gl_list_iterator_next (iter, &e, node_ptr) ? e : NULL;
}

to the global API (modulo a better name for the procedure).


Am Di., 6. Apr. 2021 um 21:20 Uhr schrieb Paul Eggert <eggert@cs.ucla.edu>:

> On 4/6/21 12:13 PM, Marc Nieper-Wißkirchen wrote:
> > gl_list_iterator_next has to return two things: An element (represented
> by
> > a const void *) and a boolean value. As elements may be NULL
>
> Ah, OK, then that's the problem. The API shouldn't allow null elements.
> They're not that useful anyway. If they really are needed for some
> lists, I suppose one could have a more-complicated API to return them
> (by setting a bool elsewhere); but usually they aren't.
>
> > So to make my original code portable C, I would have to code
> >
> > ...
> > const void *e;
> > while (gl_list_iterator_next (&i, &e, NULL))
> >   {
> >     struct foo *foo = (void *) e;
> >     ++foo->bar;
> >   }
> > ...
> >
> > The const typecast is, unfortunately, still needed to silence compiler
> > warnings
>
> Yes, that would be portable. But that cast indicates another problem
> with the API. It should return void *, not void const * (think of strchr
> as the model here). The API should discourage type-casts, since they're
> more dangerous than the alternatives.
>

[-- Attachment #2: Type: text/html, Size: 3170 bytes --]

Re: Type-safe typecasts

[Bruno Haible]

Hi Ben,

> I've found a few macros for casts useful over years. PSPP uses
> CONST_CAST and UP_CAST from the file below quite a bit:
> https://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h

That's pointing to a nice solution to the problem that casts don't warn in C.
I use to write code like

   void *p = ...;
   int *q = (int *) p;

because I want the reader of the program to be aware that there is a cast.

Paul uses to write code like

   void *p = ...;
   int *q = p;

because he wants the compiler to issue warnings if the type of p is changed
to something else.

It would be cool to have the advantages of both approaches:

   void *p = ...;
   int *q = SAFE_CAST (int *, p);

that would indicate that there is a cast AND warn if 'int *' and 'void *'
are not compatible.  Can you implement SAFE_CAST with the techniques from
PSPP, or is Marc's approach needed?

Btw, I like Marc's approach of using _Generic for warning purposes on newer
compilers, and still maintain compatibility with older compilers.

Bruno

Re: Type-safe typecasts

[Bruno Haible]

Ben Pfaff wrote:
> I've found a few macros for casts useful over years. PSPP uses
> CONST_CAST and UP_CAST from the file below quite a bit:
> https://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h

Other projects may want to use these macros as well.
That would make a great contribution to Gnulib!

Bruno

Re: Type-safe typecasts

[Bruno Haible]

Marc Nieper-Wißkirchen wrote:

> gl_list_iterator_t i = gl_list_iterator (list);
> struct foo *elt;
> while (gl_list_iterator_next (&i, (const void **) &elt, NULL))
>   ++elt->bar;

This cast is dangerous: It silences the warning "passing argument 2 of
'gl_list_iterator_next' from incompatible pointer type", and is (AFAICS)
a violation of the C strict-aliasing rule [1].

> So to make my original code portable C, I would have to code
> 
> ...
> const void *e;
> while (gl_list_iterator_next (&i, &e, NULL))
>   {
>     struct foo *foo = (void *) e;
>     ++foo->bar;
>   }

Right, this is the way it should be written.

> The const typecast is, unfortunately, still needed to silence compiler
> warnings as the Gnulib list API suffers a bit from const-poisoning when the
> actual elements are pointers actually non-const objects.

Yes. The element type could be 'const void *' everywhere or 'void *'
everywhere. Since the generic list code does not write to these pointers,
I chose 'const void *'.

Bruno

[1] https://stackoverflow.com/questions/98650/what-is-the-strict-aliasing-rule

Re: gl_list API

[Bruno Haible]

Paul Eggert wrote:
> > gl_list_iterator_next has to return two things: An element (represented by
> > a const void *) and a boolean value. As elements may be NULL
> 
> Ah, OK, then that's the problem. The API shouldn't allow null elements. 
> They're not that useful anyway.

I disagree very much with this. When you write a list type for use within
a particular program, you can say "storing null elements is not useful".
But when designing a general utility, for all kinds of programs to use,
it is inappropriate to say "storing null elements is not useful". The
program's developer decides about the representation of their data, and
if they find that NULL is a perfect representation for something, then
that's their valid choice.

> If they really are needed for some 
> lists, I suppose one could have a more-complicated API to return them 
> (by setting a bool elsewhere); but usually they aren't.

I wanted to avoid this when designing the gl_list API: to have one
API for the case that disallows NULLs, and another — more complex — API
for that case that allows NULLs. One API function for one thing. No
redundancies.

> > The const typecast is, unfortunately, still needed to silence compiler
> > warnings
> 
> Yes, that would be portable. But that cast indicates another problem 
> with the API. It should return void *, not void const * (think of strchr 
> as the model here).

strchr is a bad model here: You pass it a pointer to a read-only storage
(such as a literal string) and receive a pointer that you "can" write
to: The compiler cannot warn about
   strchr ("literal", 'e') [2] = 'x';

> The API should discourage type-casts, since they're 
> more dangerous than the alternatives.

You can't write generic containers in C that don't need type-casts,
except by adding wrapper macros that do the type casts for the user.

So far we have been lacking type-casts that warn for invalid use;
it is well possible that with the PSPP macros (or with Marc's approach
of _Generic), we can design a type-safe wrapper around gl_list.h
that is parameterized by e.g.
  #define ELEMENT_TYPE  struct foo *
  #define LIST_TYPE     foo_list_t
  #define PREFIX        fl_
and that does warning-free but safe casts between 'struct foo *' and
'const void *'.

That's the direction we shoud go. Not backwards by using bad models
like strchr().

Bruno

Re: gl_list API

[Paul Eggert]

On 4/6/21 1:28 PM, Bruno Haible wrote:
> But when designing a general utility, for all kinds of programs to use,
> it is inappropriate to say "storing null elements is not useful".

I'm afraid we'll have to disagree here. But then I don't use the API so 
my comments aren't all that important.

Re: gl_list API

[Bruno Haible]

> > But when designing a general utility, for all kinds of programs to use,
> > it is inappropriate to say "storing null elements is not useful".
> 
> I'm afraid we'll have to disagree here.

In some of my uses of the gl_list module, the element is in fact a small
integer, cast to an 'uintptr_t' and then further cast to a 'const void *'.
If the API would forbid storing a NULL pointer, my specialized container
type would need a workaround for storing the integer value 0.

Bruno

Re: gl_list API

[Marc Nieper-Wißkirchen]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

Am Di., 6. Apr. 2021 um 23:04 Uhr schrieb Bruno Haible <bruno@clisp.org>:

> > > But when designing a general utility, for all kinds of programs to use,
> > > it is inappropriate to say "storing null elements is not useful".
> >
> > I'm afraid we'll have to disagree here.
>
> In some of my uses of the gl_list module, the element is in fact a small
> integer, cast to an 'uintptr_t' and then further cast to a 'const void *'.
> If the API would forbid storing a NULL pointer, my specialized container
> type would need a workaround for storing the integer value 0.
>

NULL can also be a sensible value for "holes" in the list showing up in
algorithms that need to lazily remove elements (e.g. the list will only be
eventually compactified).

[-- Attachment #2: Type: text/html, Size: 1257 bytes --]

Re: cast macros

[Bruno Haible]

I wrote:
> So far we have been lacking type-casts that warn for invalid use;
> it is well possible that with the PSPP macros (or with Marc's approach
> of _Generic), we can design a type-safe wrapper around gl_list.h

Here is a simplified test case: The simplest container type is a box type,
that contain just one value. Can one of you complete this code, so that
it produces the warnings / no warnings as indicated?

============================================================================
typedef const void ** gl_box_t;

extern gl_box_t create_box (const void *);
extern const void *box_get_value (gl_box_t);

#include "cast.h"

/* CAST_TO_FROM (TO, FROM, EXPR)
   gives a warning if EXPR is not of type FROM,
   and returns EXPR, cast to type TO.  */

struct foo;
#define ELEMENT_TYPE struct foo *
#define CREATE_BOX(V) \
  create_box (CAST_TO_FROM (const void *, ELEMENT_TYPE, V))
#define BOX_GET_VALUE(BOX) \
  CAST_TO_FROM (ELEMENT_TYPE, const void *, box_get_value (BOX))

struct foo *a;
struct foo *b;
const void *c;
void *d;
int *i;

int
main ()
{
  (void) CAST_TO_FROM (int *, const void *, c); // no warning
  (void) CAST_TO_FROM (int *, void *, d);       // no warning
  (void) CAST_TO_FROM (int *, const void *, d); // no warning
  (void) CAST_TO_FROM (int *, void *, c);       // warning
  (void) CAST_TO_FROM (int *, void *, i);       // warning
  (void) CAST_TO_FROM (int *, char *, i);       // warning

  gl_box_t box = CREATE_BOX (a);                // no warning
  return BOX_GET_VALUE (box) == b;              // no warning
}
============================================================================

Re: cast macros

[Ben Pfaff]

On Tue, Apr 6, 2021 at 4:04 PM Bruno Haible <bruno@clisp.org> wrote:
>
> I wrote:
> > So far we have been lacking type-casts that warn for invalid use;
> > it is well possible that with the PSPP macros (or with Marc's approach
> > of _Generic), we can design a type-safe wrapper around gl_list.h
>
> Here is a simplified test case: The simplest container type is a box type,
> that contain just one value. Can one of you complete this code, so that
> it produces the warnings / no warnings as indicated?
>
> ============================================================================
> typedef const void ** gl_box_t;
>
> extern gl_box_t create_box (const void *);
> extern const void *box_get_value (gl_box_t);
>
> #include "cast.h"
>
> /* CAST_TO_FROM (TO, FROM, EXPR)
>    gives a warning if EXPR is not of type FROM,
>    and returns EXPR, cast to type TO.  */
>
> struct foo;
> #define ELEMENT_TYPE struct foo *
> #define CREATE_BOX(V) \
>   create_box (CAST_TO_FROM (const void *, ELEMENT_TYPE, V))
> #define BOX_GET_VALUE(BOX) \
>   CAST_TO_FROM (ELEMENT_TYPE, const void *, box_get_value (BOX))
>
> struct foo *a;
> struct foo *b;
> const void *c;
> void *d;
> int *i;
>
> int
> main ()
> {
>   (void) CAST_TO_FROM (int *, const void *, c); // no warning
>   (void) CAST_TO_FROM (int *, void *, d);       // no warning
>   (void) CAST_TO_FROM (int *, const void *, d); // no warning
>   (void) CAST_TO_FROM (int *, void *, c);       // warning
>   (void) CAST_TO_FROM (int *, void *, i);       // warning
>   (void) CAST_TO_FROM (int *, char *, i);       // warning
>
>   gl_box_t box = CREATE_BOX (a);                // no warning
>   return BOX_GET_VALUE (box) == b;              // no warning
> }
> ============================================================================

I spent some time experimenting and I don't know a way to do that.

One way to come close, using GCC extensions:
  #define CAST_TO_FROM(to, from, expr) ({                                 \
        _Static_assert (__builtin_types_compatible_p (typeof(expr), from)); \
        (to) (expr);                                                      \
      })
but this will give an unwanted warning for
  (void) CAST_TO_FROM (int *, const void *, d); // no warning
because the 'const' is not outermost.

Another way, without GCC extensions, is:
  #define CAST_TO_FROM(to, from, expr) ({         \
        (void) sizeof ((from) (expr) == (expr));  \
        (to) (expr);                              \
      })
but this fails to warn for either of the following because void mixes with
other types so freely in C:
  (void) CAST_TO_FROM (int *, void *, c);       // warning
  (void) CAST_TO_FROM (int *, void *, i);       // warning

Re: gl_list API

[Ben Pfaff]

On Tue, Apr 6, 2021 at 1:30 PM Bruno Haible <bruno@clisp.org> wrote:
> Paul Eggert wrote:
> > Yes, that would be portable. But that cast indicates another problem
> > with the API. It should return void *, not void const * (think of strchr
> > as the model here).
>
> strchr is a bad model here: You pass it a pointer to a read-only storage
> (such as a literal string) and receive a pointer that you "can" write
> to: The compiler cannot warn about
>    strchr ("literal", 'e') [2] = 'x';

I've gotten in the habit of having two functions in cases like this. If
strchr were designed according to the principles that I try to apply,
we'd have functions like this:

const char *strchr (const char *, int);
char *strchr_rw (char *, int);

The implementation of strchr is then just a wrapper around the
strchr_rw():

const char *
strchr (const char *s, int c)
{
    return strchr_rw ((char *) s, c);
}

although if I'm doing this in PSPP I would probably use CONST_CAST:
    return strchr_rw (CONST_CAST (char *, s), c);