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

Issue 2531 fix vs2022 build #2556

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

atamagaii
Copy link

@atamagaii atamagaii commented Jan 21, 2025

Okay my plan here is to take a multi stage approach to getting the latest hash_table8 moved into the project.

  1. [This PR] Fix all VS2022 build errors including the original shadowing error mentioned in MSVC 2022 17.12.1 configure.py --bootstrap fails on emhash/hash_table8.hpp #2531
  2. Target c++14
  3. Pull in the latest version of hash_table8

Changes

  1. Fixed the variable shadowing in hash_table8. The justification for fixing here instead of taking version 1.7 directly from the emhash project is that we are targeting c++11 and can't support the latest version.
  2. We had defined the macro #define unlink _unlink but the compiler still viewed 'unlink' as a POSIX function. I created a function that injects the correct version of unlink depending on the platform.
  3. Lots of sign mismatch warnings. Logically something like EXPECT_EQ(list.size(), 1); makes sense, but the compiler doesn't like it. Ideally we would be matching all of our assertions to the exact type. But realistically EXPECT_EQ(list.size(), size_t(1)); and EXPECT_EQ(list.size(), 1u); are going to be equivalent for small values.

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

Successfully merging this pull request may close these issues.

1 participant