-
Notifications
You must be signed in to change notification settings - Fork 28
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
Generate __hash__ on python as well #76
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제 생각에는 해시 충돌에 관한 케이스에 대해서는 확률이 충분히 낮기 때문에 실제에서 큰 문제가 되지는 않으리라고 봅니다. 하지만 CI 등에서 반복적으로 빌드를 하다보면 그런 일이 일어날 수도 있습니다.
@@ -200,6 +200,12 @@ compileUnionTag source parentname typename' fields = do | |||
slots = if length tagNames == 1 | |||
then [qq|'{head tagNames}'|] `T.snoc` ',' | |||
else toIndentedCodes (\n -> [qq|'{n}'|]) tagNames ",\n " | |||
hashTuple = if null tagNames | |||
then "self.__nirum_tag__" | |||
else [qq|tuple([self.__class__, {attributes}])|] :: T.Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해시할 때 타입 정보 안 넣어도 됩니다. 파이썬 hash()
가 원래 같은 타입 안에서만 충돌이 안 나면 (확률적으로 충분히 낮게 나면) 되거든요. 내부적으로 hash()
를 사용하는 dict
/set
등이 버킷 나눌 때 hash(v)
만 가지고 나누는 게 아니라 (type(v), hash(v))
쌍을 기준으로 나눕니다. 따라서 저희가 이걸 할 필요가 없어요.
그리고 (a, b, c)
같은 식으로 써도 될텐데 tuple([a, b, c])
식으로 출력되게 만드셨네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tuple()
로 한거는 사실 위에 코드에도있지만 argument가 한개일때 따로 trailing comma를 넣는 처리를 하기싫어서 저렇게 해놓긴했습니다.
tuple([a])
이건괜찮지만, (a)
이건사실 tuple
로 평가되질 않는 문제가 ㅜㅜ..
@@ -325,7 +334,7 @@ class $className: | |||
self.value == other.value) | |||
|
|||
def __hash__(self) -> int: | |||
return hash(self.value) | |||
return hash((self.__class__, self.value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지입니다. 타입은 안 넣어도 됩니다.
@@ -360,6 +372,7 @@ class $className(enum.Enum): | |||
@classmethod | |||
def __nirum_deserialize__(cls: type, value: str) -> '{className}': | |||
return cls(value.replace('-', '_')) # FIXME: validate input | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 줄바꿈은 잘못 섞여들어간 건가요, 아니면 일부러 한칸 더 넣은 건가요?
@@ -381,6 +394,9 @@ compileTypeDeclaration src TypeDeclaration { typename = typename' | |||
toNamePair | |||
[name | Field name _ _ <- toList fields] | |||
",\n " | |||
hashTuple = [qq|(self.__class__, {attributes})|] :: T.Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지입니다. 타입은 안 넣어도 됩니다.
tT decl [q|hash(Point(left=3, top=14)) == | ||
hash(Point(left=3, top=14))|] | ||
tT decl [q|hash(Point(left=3, top=14)) != | ||
hash(Point(left=1, top=592))|] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사실 이 테스트는 항상 성공한다고 보장할 수가 없습니다. 해시 함수의 특성을 생각해봅시다:
해시 함수의 가장 기본적인 성질은 두 해시 값이 다르다면 원래의 데이터도 다르다는 것이다. 이 특징은 해시 함수가 결정적이기 때문이다. 반대로 해시 함수는 단사 함수가 아니다. 같은 해시 값을 갖더라도 원래의 입력값이 같다는 것을 시사하지만 보장해주지는 않는다. 원래 입력의 한 비트만 바뀌더라도 해시 함수의 성질로 인해 해시 값은 크게 달라진다.
hash(x)
와 hash(y)
의 결과가 다르다면 x
와 y
도 다르다는 뜻입니다. 하지만 hash(a)
와 hash(b)
의 결과가 같다고 해서 a
와 b
도 서로 같으리라 보장할 수는 없습니다. (인용한 것처럼 시사할 수는 있습니다.) 반대로 얘기하면 a
와 b
가 다른 값이지만 우연히 hash(a)
와 hash(b)
가 같은 결과를 내는 해시 충돌을 만날 수도 있습니다. 따라서 위 테스트에서 아래쪽 단언문은 항상 성공을 보장하진 않습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 감사합니다
tT decl [q|hash(WesternName(first_name='foo', middle_name='bar', | ||
last_name='baz')) != | ||
hash(WesternName(first_name='hello', middle_name='bar', | ||
last_name='baz'))|] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지입니다. 해시 충돌이 있을 수 있으므로 이 단언문은 항상 성공하지는 않습니다.
tT decl [q|hash(WesternName(first_name='foo', middle_name='bar', | ||
last_name='baz')) != | ||
hash(WesternName2(first_name='foo', middle_name='bar', | ||
last_name='baz'))|] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지입니다. 해시 충돌이 있을 수 있으므로 이 단언문은 항상 성공하지는 않습니다.
f06b5a2
to
700ab63
Compare
implement #75