|Summary:||Fix lang/ruby21 installed header to fix signed shift warnings for other ports (e.g. devel/rubygem-thrift)|
|Product:||Ports & Packages||Reporter:||Dimitry Andric <dim>|
|Component:||Individual Port(s)||Assignee:||freebsd-ruby mailing list <ruby>|
|Severity:||Affects Some People||CC:||mmoll, sunpoet|
|Bug Depends on:|
Description Dimitry Andric 2015-08-25 18:47:18 UTC
Created attachment 160356 [details] Fix signed left-shifting in lang/ruby21's public ruby.h During the exp-run in bug 201377, it was found that devel/rubygem-thrift gives errors with a recent clang 3.7.0 snapshot: http://package18.nyi.freebsd.org/data/headi386PR201377-default/2015-08-20_15h42m20s/logs/errors/rubygem-thrift-0.9.1,1.log This is because it includes ruby.h from the lang/ruby21 port, which uses this macro: #define INT2FIX(i) ((VALUE)(((SIGNED_VALUE)(i))<<1 | FIXNUM_FLAG)) Unfortunately, left-shifting negative 'i' values is undefined behavior, so clang warns about this. I changed the macro to: #define INT2FIX(i) ((VALUE)(((VALUE)(i))<<1 | FIXNUM_FLAG)) which avoids the undefined behavior by casting to VALUE (which is unsigned) first. I ran all ruby21 tests before and after this fix, and I got "1 failures, 4 errors" in both cases, so no regressions. However, since this is a public ruby header, I can imagine that this is a change that makes some people nervous. So if ruby maintainers prefer to fix this in the devel/rubygem-thrift port instead, for example by squelching the warning, please let me know.
Comment 1 Michael Moll 2015-09-06 22:18:39 UTC
https://github.com/ruby/ruby/commit/1efb3c31b731e99627bbc0da13dfd3463bb67c67 did change this i upstream to #define INT2FIX(i) (((VALUE)(i))<<1 | FIXNUM_FLAG) could you check if that solves the problem with clang 3.7.0?
Comment 2 Dimitry Andric 2015-09-07 20:38:12 UTC
(In reply to Michael Moll from comment #1) > https://github.com/ruby/ruby/commit/1efb3c31b731e99627bbc0da13dfd3463bb67c67 > did change this i upstream to > #define INT2FIX(i) (((VALUE)(i))<<1 | FIXNUM_FLAG) > could you check if that solves the problem with clang 3.7.0? Yes, that is fine too. Even better that it comes from upstream. :) I applied the complete diff you mentioned, but it may be a bit overkill. It should be enough to just patch the public ruby.h header. Btw, I think this also applies to the other ruby ports?
Comment 3 Michael Moll 2015-09-07 20:53:45 UTC
Let's stick with the one line patch to ruby.h then, if that's sufficient. :) It would also be applicable to lang/ruby20, as lang/ruby22 already includes the whole upstream change.
Comment 4 commit-hook 2015-09-20 21:31:33 UTC
A commit references this bug: Author: mmoll Date: Sun Sep 20 21:30:41 UTC 2015 New revision: 397464 URL: https://svnweb.freebsd.org/changeset/ports/397464 Log: fix Ruby 2.0 and 2.1 header for clang 3.7 2.2 already had this change in upstream PR: 202660 Submitted by: dim (different version) Obtained from: Ruby SVN repository (r47996) Changes: head/lang/ruby20/files/patch-include_ruby_ruby.h head/lang/ruby21/files/patch-include_ruby_ruby.h
Comment 5 Michael Moll 2015-09-20 21:32:14 UTC