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

Hash improvement for complex, str and long. #797

Merged
merged 2 commits into from
Aug 5, 2015
Merged

Conversation

Daetalus
Copy link
Contributor

@Daetalus Daetalus commented Aug 4, 2015

Change

  • Pypy parser could not handle complex literal correctly, it always treat complex as 0.0j. Fix it. The code originally write by Marius.
  • Add hash function to complex. Already tested. (But did not enable test_hash, please see below)
  • Fix the wrong mpz usage in long constructor.
  • Treat the hash value of empty string as Zero, like CPython did.

Note
After read CPython code. CPython str hash function will check PYTHONHASHSEED enviroment varialble in random.c file. But the Pyston DJB based str hash function didn't support it. I did some experiments try to fix it. But seems I couldn't use random number in str hash function (types.h). Pyston will report error:

pyston: ../../src/runtime/builtin_modules/thread.cpp:189: void pyston::setupThread():
Assertion `thread_module' failed.

long hashreal, hashimag, combined;
hashreal = _Py_HashDouble(self->real);
if (hashreal == -1)
return boxInt(-1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be throwCapiException() -- returning -1 is the capi way for hash functions to specify that they threw an exception.

kmod added a commit that referenced this pull request Aug 5, 2015
Hash improvement for complex, str and long.
@kmod kmod merged commit 52e385e into pyston:master Aug 5, 2015
@kmod
Copy link
Collaborator

kmod commented Aug 5, 2015

awesome thanks :)

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.

2 participants