{"action":"created","issue":{"url":"https://api.github.com/repos/manastech/crystal/issues/559","labels_url":"https://api.github.com/repos/manastech/crystal/issues/559/labels{/name}","comments_url":"https://api.github.com/repos/manastech/crystal/issues/559/comments","events_url":"https://api.github.com/repos/manastech/crystal/issues/559/events","html_url":"https://github.com/manastech/crystal/issues/559","id":69554899,"number":559,"title":"Inconsistent modulo","user":{"login":"BlaXpirit","id":371383,"avatar_url":"https://avatars.githubusercontent.com/u/371383?v=3","gravatar_id":"","url":"https://api.github.com/users/BlaXpirit","html_url":"https://github.com/BlaXpirit","followers_url":"https://api.github.com/users/BlaXpirit/followers","following_url":"https://api.github.com/users/BlaXpirit/following{/other_user}","gists_url":"https://api.github.com/users/BlaXpirit/gists{/gist_id}","starred_url":"https://api.github.com/users/BlaXpirit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BlaXpirit/subscriptions","organizations_url":"https://api.github.com/users/BlaXpirit/orgs","repos_url":"https://api.github.com/users/BlaXpirit/repos","events_url":"https://api.github.com/users/BlaXpirit/events{/privacy}","received_events_url":"https://api.github.com/users/BlaXpirit/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":19,"created_at":"2015-04-20T10:20:40Z","updated_at":"2015-04-25T05:09:21Z","closed_at":null,"body":"```ruby\r\n>> (-7) % 5\r\n# Crystal: -2\r\n# Ruby: 3\r\n>> (-7.0) % 5.0\r\n# Crystal: undefined method '%' for Float64\r\n# Ruby: 3.0\r\n>> (-7).modulo(5)\r\n# Crystal: -2\r\n# Ruby: 3\r\n>> (-7).remainder(5)\r\n# Crystal: undefined method 'remainder' for Int32\r\n# Ruby: -2\r\n>> require \"big_int\"; (-7.to_big_i) % (5.to_big_i)\r\n# Crystal: 3\r\n```\r\n\r\nI suggest leaving `%` as is for performance, but making `modulo` wrap around like Ruby (possible implementation: `(a % b + b) % b`).\r\n\r\nWouldn't be bad to add `remainder` which is the same as current `%`."},"comment":{"url":"https://api.github.com/repos/manastech/crystal/issues/comments/96131985","html_url":"https://github.com/manastech/crystal/issues/559#issuecomment-96131985","issue_url":"https://api.github.com/repos/manastech/crystal/issues/559","id":96131985,"user":{"login":"ssvb","id":78642,"avatar_url":"https://avatars.githubusercontent.com/u/78642?v=3","gravatar_id":"","url":"https://api.github.com/users/ssvb","html_url":"https://github.com/ssvb","followers_url":"https://api.github.com/users/ssvb/followers","following_url":"https://api.github.com/users/ssvb/following{/other_user}","gists_url":"https://api.github.com/users/ssvb/gists{/gist_id}","starred_url":"https://api.github.com/users/ssvb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ssvb/subscriptions","organizations_url":"https://api.github.com/users/ssvb/orgs","repos_url":"https://api.github.com/users/ssvb/repos","events_url":"https://api.github.com/users/ssvb/events{/privacy}","received_events_url":"https://api.github.com/users/ssvb/received_events","type":"User","site_admin":false},"created_at":"2015-04-25T05:09:21Z","updated_at":"2015-04-25T05:09:21Z","body":"Here is the @asterite's benchmark program with a correctness test added:\r\n```ruby\r\nrequire \"benchmark\"\r\n\r\n# A slow reference implementation\r\ndef modulo_ref(a, b)\r\n  a = a.to_i64\r\n  b = b.to_i64\r\n  (a % b + b) % b\r\nend\r\n\r\n# 'a' and 'b' need to be two's complement signed integers of the same size\r\ndef modulo(a, b)\r\n  if b == 0\r\n    raise DivisionByZero.new\r\n  elsif (a ^ b) >= 0\r\n    a.unsafe_mod(b)\r\n  else\r\n    a = a.unsafe_mod(b)\r\n    a == 0 ? 0 : a + b\r\n  end\r\nend\r\n\r\n# Correctness test\r\n\r\nmacro modulo_correctness_test_for_type(typename)\r\n  ({{typename}}::MIN .. {{typename}}::MAX).each do |i|\r\n    ({{typename}}::MIN .. {{typename}}::MAX).each do |j|\r\n      next if j == 0\r\n      next if i == {{typename}}::MIN && j == -1\r\n      abort \"problems with #{i} % #{j}\" if modulo_ref(i, j) != modulo(i, j)\r\n    end\r\n  end\r\nend\r\n\r\nmodulo_correctness_test_for_type(Int8)\r\n\r\n# Benchmark\r\n\r\nFROM = -10_000\r\nTO = 10_000\r\n\r\na1 = 0\r\na2 = 0\r\n\r\nBenchmark.bm do |x|\r\n  x.report(\"%\") do\r\n    a = 0\r\n    (FROM..TO).each do |i|\r\n      (FROM..TO).each do |j|\r\n        next if j == 0\r\n        a += i % j\r\n      end\r\n    end\r\n    a1 = a\r\n  end\r\n  x.report(\"modulo\") do\r\n    a = 0\r\n    (FROM..TO).each do |i|\r\n      (FROM..TO).each do |j|\r\n        next if j == 0\r\n        a += modulo(i, j)\r\n      end\r\n    end\r\n    a2 = a\r\n  end\r\nend\r\n\r\npp a1\r\npp a2\r\n```\r\nThe output on my system:\r\n```\r\n             user     system      total        real\r\n%        0.010834   0.000000   0.010834 (  7.103003)\r\nmodulo   0.011841   0.000000   0.011841 (  7.768067)\r\n```\r\nA few notes:\r\n\r\n* The ```(a % b + b) % b``` is not really correct unless 'a' and 'b' are casted to a larger type. For example, ```a = -127_i8 ; b = -128_i8 ; p (a % b + b) % b``` prints ```1``` instead of ```-127```\r\n* There is a signed integer overflow problem to watch out, see #575"}}