cnbatch / dynarray

A header-only library, VLA for C++ (≥C++14). Extended version of std::experimental::dynarray
BSD 3-Clause "New" or "Revised" License
14 stars 0 forks source link

Include guard macro uses a reserved name #2

Closed jwakely closed 9 months ago

jwakely commented 9 months ago

https://github.com/cnbatch/dynarray/blob/7d86b2f743d3edb1009d24953bab92539521b86c/dynarray.hpp#L55

_VLA_DYNARRY_ is a reserved name, which makes this header undefined according to the standard. There's no reason you need a leading underscore for that macro, you can just use VLA_DYNARRY or VLA_DYNARRY_HPP_INCLUDED or something like that.

cnbatch commented 9 months ago

The macros have been updated to prevent the use of reserved names.

Thank you for your valuable suggestion!

jwakely commented 9 months ago

Nice, thanks!

LocalSpook commented 3 months ago

Commit d9189b4 didn't fix the issue. The new names (for example, _VLA_HEADER_DYNARRAY_HPP_) still have leading underscores.

jwakely commented 3 months ago

Right, that's just as bad (I didn't check the actual code, just assumed what was said here was correct).

The problem is not that the names don't end in "HPP" it's that the start with an underscore followed by an uppercase letter. _VLA_DYNARRAY is bad, VLA_DYNARRY is good. If you want so use XXX_HEADER_XXX_HPP that's fine, but it can't use a leading underscore.

cnbatch commented 3 months ago

Truely sorry about that. Please try the new version, I have now deleted the underscores at the beginning of the macros.