-
Notifications
You must be signed in to change notification settings - Fork 175
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
Fix colliding mocking context ids #296
Conversation
Ok... I need to rework constExprHash so no C++14 features are used |
I haven't tried to see if it give the same result, but maybe something like this ? constexpr size_t hash(const char* str, size_t val = 14695981039346656037ULL)
{
return *str == '\0'
? val
: hash(++str, val ^ static_cast<size_t>(*str) * 1099511628211ULL);
} https://godbolt.org/z/7vqYqPTPh EDIT: It probably won't give the same result because the order of evaluation of the parameters is unspecified, maybe EDIT 2: And it seems that the multiply operator take precedence over the bitwise XOR operator, yet again something that makes it non conforment (but easily fixable with parenthesis). |
You need to add the new test files to https://github.com/eranpeer/FakeIt/blob/master/build/sources.mk as well, otherwise they won't run for GCC / Clang. |
Coverage decreased by 4.0e-05% 😮 😆 The Visual Studio build yields lots of |
I guess my .h/.cpp helper files need to be added to the sources.mk as well |
You're right, but only the .cpp. |
Thanks for the fix, I'll try to look more into it next week, I'll do some benchmark because I'm curious if it affect compilation time or not. |
I didn't see any measurable difference in compilation time with the fix for the FakeIt unit tests with GCC. I guess it's because our test files are small and the includes take a lot of time to compile, maybe with a test file that is a lot bigger (and pre-compiled header ?) we could notice a difference, or maybe the overhead is so insignificant that we wouldn't notice anything anyway. Either way it looks good to me. |
If using precompiled headers (e.g. with VS) it might make the difference more noticeable (if fakeit.h is added to that). |
Superseded by #336. I went with a different approach that I explained in the PR. |
Fixes #294