Issue 1869 duplicate fuctions break ksm #1871
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now the function hash to identify unique functions will make a hash from a string that not only includes the body of the function, but also the line/col information prepended to it. (Thus two identical function bodies located in different places will get a different hash code and be treated differently.)
I just realized a potential failure case in the very odd specific instance where the same function body exists in two places that when line and col get concatenated happen to form the same string. i.e. line 13, column 2, when concatenated make the string 132. But so would line 1, column 32. It would also be 132. So I added a few chars to the format string to distinguish them. Now line 13, column 2 will be L13C2 and line 1, column 32 will be L1C32.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1869 : Added line/col to the string that gets hashed to determine a function's identifying hash code. This makes it so if the same body appears in two places it won't quite be the same hash.
One small downside is that previously if you had said
lock throttle to 0.in two places in the same scope, they'd have only made 1 actual function for it and it would do double-duty for both of them. This optimization had to go away to fix this bug - now there will still be 2 redundant instances in that case.In fact, I'm now beginning to wonder if we shouldn't do a refactor later that just completely removes the hash system altogether. The entire point of it was to deliberately quash two instances of the same function body together for efficiency. If we removed that, then why use the hash? But that's something I can look at later - it does work with this PR as-is.