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

Conversion warnings #15

Open
EricOpitz-434 opened this issue Oct 16, 2024 · 1 comment
Open

Conversion warnings #15

EricOpitz-434 opened this issue Oct 16, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@EricOpitz-434
Copy link

Thanks for this beautiful library! I have integrated the library into my project and I have two comments:

  1. When I activate the gcc conversion warnings (-Wconversion) I get a bunch of warnings from cc.h.
  2. I'm using a map with a key of type const char *. I had to manually add this key type using CC_CMPR and CC_HASH. As I think this is a pretty common case, maybe you could consider adding this to the types that are supported out of the box.
@JacksonAllan
Copy link
Owner

Hi EricOpitz! I'm glad you're getting some use out of the library.

Regarding your points:

  1. The warning levels that CC was specifically designed to support are -Wall and -Wpedantic. I considered -Wconversion during early development, but GCC was complaining a lot about the passing of signed integers into the default hash and comparison functions for corresponding unsigned integer types, and I felt that defining a separate set of default (pass-through) functions for signed integers just to silence this optional warning was unnecessary. However, this is a rather minor change that I'm willing to make. I see now that -Wconversion also causes a lot of warnings about the conversion to double when a hash table's bucket count is multiplied by its maximum allowable load factor. All those warnings can probably be silenced by adding explicit casts. So in summary, I'll try to support -Wconversion in the next update, unless there are unforeseen complications.

  2. CC doesn't support const-qualified keys or elements in the general case because some containers—namely vectors, maps, and sets—move this data around internally. Obviously, this concern doesn't apply when we're talking about const-qualified pointers like const char *. Currently, string keys and elements are only supported by default via char *, so the user needs to manually cast away the const, where necessary, when inserting strings, which is hardly ideal for a library that advertises itself as "ergonomic" and is also bad from the standpoint of type safety. (Interestingly, GCC doesn't complain when the inserted key or element is a string literal.) It seems like addressing this issue will only require adding default hash and comparison functions for const char *, so again, I will do this in the next update. This issue is, by the way, the same issue flagged in Verstable, which is the basis of CC's hash-table implementation.

@JacksonAllan JacksonAllan added the enhancement New feature or request label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants