eteran / c-vector

A dynamic array implementation in C similar to the one found in standard C++
MIT License
737 stars 109 forks source link

Modify local var names to avoid shadowing. #35

Closed ShujianQian closed 2 years ago

ShujianQian commented 2 years ago

Example of Shadowing Problem

With the original definition, the following code

cv_cap = 10;
cvector_reserve(vec, cv_cap);

would be expanded into

do {
        size_t cv_cap = cvector_capacity(vec);
        if (cv_cap < (cv_cap)) {
                cvector_grow((vec), (cv_cap));
        }
} while (0)

which is incorrect.

Changing cv_cap to __cv_cap reduces the chance of such issues.

eteran commented 2 years ago

Hmm, two thoughts.

  1. This PR has conflicts with your previous PR, so please resolve those first.
  2. in C and C++, "All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use." So using a double underscore is considered a big no-no.

I'm certainly open to suggestion on this one, but because this is using macros, it's hard to resolve without resorting to things which are typically out of bounds.

ShujianQian commented 2 years ago

I can definitely see your point there. I am following the convention commonly found in the Linux kernel and other C codebases. Here are some examples:

This issue seems to argue that name clashes with double underscores are rather uncommon.

Other options I can think of include:

eteran commented 2 years ago

I am in favor of either the prefix or underscore suffix solutions. Either one seems to reasonably avoid name collisions while avoiding what is technically undefined behavior.

I certainly understand that the linux kernel uses double underscores, but the kernel is so tightly coupled with the compiler that I think it is fair for them to use it. Redis... well, I would advise them not to do that ;-).

Anyway, If you update your PR to use any of the two suggested solutions, I'm happy to merge!

ShujianQian commented 2 years ago

Sounds good! I changed the variable names to use a double underscore suffix.