Convenience macros

[podom]

Nice library. I'm finding it very useful. I implemented some macros to make things a little more convenient, if you'd like to integrate them.

[Chamber6821]

It seems you added or removed a trailing blank line. I don't think this is the desired change.

[podom]

Take another look at the full diff, please. I undid all the formatting changes you mentioned.

[Mashpoe]

I like these changes from an API design perspective, but I have a few suggestions for their implementations.

The main thing is that, as far as I know, each of the vector macros only ever evaluates the vec (or vec_addr) argument once, while some of the macros in this PR evaluate it more than once. Even though I would assume that most experienced C devs are aware of the limitations of the preprocessor, the macros in this library look very similar to regular function calls that are also used in this library. This is kind of important, because some code won't work properly or will be considerably slower than expected if the arguments are evaluated more than once. If someone makes a function that returns a random vector (I know this will probably will happen very rarely, if at all), they wouldn't be able to directly pass the result to vector_end() because two different vectors would be used in the calculation. A more realistic example might be a function that returns a vector, but is costly to call. Even if you know it's a macro, it's still nice to not need an extra temporary variable.

So my first suggestion would be to make helper functions for some of these. Here is an example for vector_end():

#define vector_end(vec) ((typeof(vec))_vector_end(vec, sizeof(*vec)))
vector _vector_end(vector vec, vec_type_t type_size)
{
  return (vector)((unsigned char*)vec + (type_size * vector_size(vec)));
}

The only major drawback I can see of doing it this way is that you would have to make a separate version for when typeof() is missing (maybe you could add that __typeof__ definition back in).

It would be a bit more difficult to do this for vector_foreach(), but I believe I may have a working solution. I have not tested it for vectors of pointers, but I think it should work for all cases:

// the (void*) is there because the declarator sequence removes all pointer info from the type,
// so (void*)item is just used to get a valid memory address that we can increment.
#define vector_foreach(item, vec) \
  for (typeof(vec) _item = (vec), _end = vector_end(_item); _item != _end; ++_item) \
    for (item = _item, *_loop_once = (void*)_item; (void*)_item == _loop_once; ++_loop_once)

Maybe there's a much better way to do this, but it's a suggestion.

Another solution to the "multiple evaluation" problem would be to add a section in the README warning about all of this, but I don't know if I like that solution as much.

Thoughts?

[podom]

The joys of C macros! Thanks for your thoughtful reply.

First, the initial PR I submitted had the unintentional formatting changes, and I meant to move the__typeof__ #define out of vec.h. However, maybe you'd like to consider including it? It's non-standard, but it's been available in gcc and others (clang?) for quite a while. I had it in there because the project I'm working on is using the Arm GNU Toolchain version 12, which doesn't seem to support the typeof operator but does include __typeof__.

Regarding the double evaluation, your suggestion is the best standard workaround I can think of. I had to think about it for a minute, but typeof and sizeof are compile-time operators and won't result in evaluation of the macro parameter at run-time.

One could use a statement expression:

// Based on non-standard gcc statement expressions
#define vector_end(vec) ({typeof(vec) _vec = (vec); _vec + vector_size(_vec);})

But that's another gcc extension, and I know it wouldn't be available in MSVC and others. Probably not a good idea for an open-source library!

I'll take a longer look at your vector_foreach, and when I have a chance I'll push an update for consideration.

[Mashpoe]

For the typeof definition, you could do a preprocessor check similar to the existing ones in vec.h, for example:

#if _MSC_VER != 0 && __STDC_VERSION__ < 202311L
#define typeof __typeof__
#endif

But this would have to be worked into the other preprocessor checks.

I also think I have a much more robust version of the vector_foreach() macro:

#define vector_foreach(item, vec) \
for (unsigned char* _item = (void*)(vec), *_end = (void*)vector_end((typeof(vec))_item), *_curr = _item; _item != _end; _curr = _item) \
  for (item = (typeof(vec))_item; _curr == _item; _item += sizeof(*vec))

The _curr variable serves the same purpose as the _cont variable from your version. I think this version will actually be much faster because of the _end variable, which eliminates the need to call vector_size() each loop. unsinged char* is used in the outer loop so we can avoid all of the weird type issues, and from the limited tests I've done, this handles vectors of pointers without any issues.