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

change hashing function to return unsigned int #561

Merged
merged 1 commit into from Oct 24, 2016

Conversation

@thirdwing
Copy link
Member

@thirdwing thirdwing commented Oct 22, 2016

No description provided.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 22, 2016

Looks good to me!

@thirdwing thirdwing force-pushed the thirdwing:iss391_patch2 branch from eb45c80 to a7cda55 Oct 23, 2016
@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 24, 2016

Any other thumbs up besides my overused one?

@coatless
Copy link
Contributor

@coatless coatless commented Oct 24, 2016

Change context:

int => unsigned int since the hashing macro is defined to return unsigned int

@coatless
Copy link
Contributor

@coatless coatless commented Oct 24, 2016

Looks like int addr missed a change over in add_value_get_index() within SelfHash.h and add_value() within IndexHash.h

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 24, 2016

Yes, also see the now closed (because too "jumbo") PR #556

@thirdwing thirdwing force-pushed the thirdwing:iss391_patch2 branch from a7cda55 to 4f53bc2 Oct 24, 2016
@thirdwing
Copy link
Member Author

@thirdwing thirdwing commented Oct 24, 2016

I have updated the code.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 24, 2016

In it goes!

@eddelbuettel eddelbuettel merged commit adae3fe into RcppCore:master Oct 24, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@thirdwing thirdwing deleted the thirdwing:iss391_patch2 branch Oct 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.