Some small dialyzer progress #904
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
Pair-programmed with @reiddraper bitcask:delete only ever returns `ok`.
Pair-programmed with @reiddraper Dialyzer found a bug with this function, and it appears to be completely unused in Riak. It was introduced in 39343fa. In the creation of the record, ref is set to undefined, which will crash the underlying call to Bitcask when it tries to grab information of of the process dictionary.
Pair-programmed with @reiddraper Dialyzer was treating the fields of the KV_INDEX_Q that were of the same type as continuation() as opaque and getting mad about record construction.
Pair-programmed with @reiddraper Use the eleveldb:db_ref() type instead of assuming it's a reference() type.
Pair-programmed with @reiddraper We create a magic value with {0, 0} as the value, so the type spec must account for this.
Pair-programmed with @reiddraper There is commented code from consistent_put that talks about adding this in the future, so we put a comment on the receiving end of this function saying the same.
Pair-programmed with @reiddraper Dialyzer caught that the bucket will always be a binary, so there is no need to convert it. Hopefully there is no data on disk that uses atoms for buckets...
Pair-programmed with @reiddraper * Change the expected type of allow_mult * Change the error reason type to allow strings
Pair-programmed with @reiddraper Previously we were pattern matching a two-tuple where the value returned was a proplist. This change now treats the value correctly as a proplist. Note, this is still an issue in 1.4, since this was forward-ported from the 1.4 series.
Pair-programmed with @reiddraper The accumulator used internally in merge_contents is a three-tuple, not a two-tuple.
Pair-programmed with @reiddraper * db_ref is an opaque type, we don't know it's a reference * dialyzer claims the last clause of fold_keys_fun is completely subsumed by the previous clauses. If it's not, we'll throw an exception either way with a bad match.
Pair-programemd with @reiddraper
Pair-programmed with @reiddraper
Pair-programmed with @reiddraper
Pair-programmed with @reiddraper The caller of this function handles the case where the result of Module:Function does not return a list, so the type spec should reflect that
Pair-programmed with @reiddraper
riak_kv_entropy_info:handle_index_info is not exported. It was being called once internally, with an argument that was ignored in its implementation. This commit simply removes that argument.
update_hashtree is called with both binaries and riak objects.
|
Blah, made a mistake in a commit. I'll be force-pushing here shortly. |
|
@reiddraper where possible could you also include the warnings fixed by each commit? @cmeiklejohn and I started doing that so it's more clear. |
diff_index_specs clearly accepts 'undefined' as an argument because it calls index_data, which pattern matches on undefined in its definition.
Fixes the return value of riak_object:from_binary
Sure. Do you want me to go back and amend the commits? Or just going forward? |
|
I think if/once this is merged, I'll file separate issues for dialyzer work on the remaining modules, which, at first glance, required more domain expertise. |
|
Just going forward is fine. I'll take the review of this now. |
|
Left some comments, 👍 otherwise. |
|
@cmeiklejohn have all of the review comments been handled? If so, ready to merge? |
|
Merge away. |
reiddraper
added a commit
that referenced
this pull request
Apr 16, 2014
Some small dialyzer progress
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.
There are still tons of dialyzer errors not fixed by this PR, but I figured it was better to merge things early, particularly since the commits are not related to each-other. I'll add some comments if I think there are commits that need extra review by the original author.