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

Incomplete type analysis? #6

Open
P-p-H-d opened this issue Mar 30, 2023 · 2 comments
Open

Incomplete type analysis? #6

P-p-H-d opened this issue Mar 30, 2023 · 2 comments

Comments

@P-p-H-d
Copy link

P-p-H-d commented Mar 30, 2023

For the following program which is faulty (it gives an integer instead of a structure as argument of the map for the insert call),

#include <stdio.h>
#include <string.h>
#include "cc.h"

typedef struct { int x, y, z; } mp;
#define CC_CMPR mp, { return memcmp( &val_1, &val_2, sizeof (mp)); }
#define CC_HASH mp, { return val.x ^ val.y ^ val.z; }
#include "cc.h"

int main( void )
{
  map(mp, mp) map;
  init(&map);
  if (!insert(&map, 12, 12)) {
    abort();
  }
  cleanup(&map);
  return 0;
}

the compilation succeeds with some "missing braces around initializer [-Wmissing-braces]" warnings
whereas I was expecting a compilation failure or at least a warning about incompatible pointer.

@JacksonAllan
Copy link
Owner

JacksonAllan commented Mar 30, 2023

Thanks!

The issue stems from the following macro:

#define CC_MAKE_LVAL_COPY( ty, xp ) *( ty[ 1 ] ){ xp }

This macro is used to duplicate API macro arguments (elements or keys) and pass a pointer to the duplicate into a container function. In other words, it mimics by-value-argument-passing in a manner that allows us to still circumvent C's type system by using void pointers. The result is that users can pass l-values and r-values into API macros, just like regular functions, instead of only l-values whose address can be taken.

The compiler is rightfully warning that ( mp[ 1 ] ){ 12 } should instead by ( mp[ 1 ] ){ { 12 } }. In other words, 12 should be a braced initialization list and not a stand-alone integer. This warning would be relevant to the insert macro were it not for the fact that the commas in any initializer list will mess with the macro's attempt to parse its arguments.

I think compilers should generate an error here, especially with -Wall and -Wpedantic, because ( mp[ 1 ] ){ 12 } is invalid syntax. But for some reason both GCC and Clang prefer warnings.

I'm not sure there's a way to force an error, but I'll think about, especially because I'm already in the process of refactoring the library for better compile speed. But a warning might have to suffice. There is at least one other instance wherein the library generates warnings for incorrect usage where an error would be better (namely passing a cntr argument with side effects), so the overarching policy so far has been to generate at least a warning for incorrect usage and an error where possible.

@JacksonAllan
Copy link
Owner

By the way, I should also point out that your hash function return memcmp( &val_1, &val_2, sizeof (mp)); isn't strictly portable because it assumes that int contains no padding bits (consider instead comparing each member with == or using uint32_t). But in practice, this isn't likely to cause problems.

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