Skip to content
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

Add float_warnings patches for Ruby 2.0.0p64[5 7 8], 2.1.[8 9 10], 2.2.[4 5 6 7 8], 2.3.[0 1 2 3 4 5] and 2.4.[0 1 2] #4201

Merged
merged 8 commits into from
Oct 10, 2017

Conversation

knugie
Copy link
Contributor

@knugie knugie commented Oct 8, 2017

Latest float_warnings patches.

Passing tests:
# Each of the following `ruby -e` lines prints:
#-e:1: warning: initializing BigDecimal with an instance of Float.
#-e:1: warning: calling .to_d on an instance of Float.

for MAJOR in "2"; do
  for MINOR in "0.0"; do
    for PATCH in "247" "353" "451" "481" "576" "594" "598" "645" "647" "648"; do
      rvm use $MAJOR.$MINOR-p$PATCH-float_warnings --patch patches/ruby/$MAJOR.$MINOR/p$PATCH/float_warnings/01-bigdecimal_float_warning.patch --install
      ruby -e "require 'bigdecimal'; require 'bigdecimal/util'; BigDecimal.new(123.4, 2) && 7.1.to_d"
      rvm remove $MAJOR.$MINOR-p$PATCH-float_warnings
    done
  done
  for MINOR in "1"; do
    for PATCH in "0" "1" "2" "3" "4" "5" "6" "7" "8" "9" "10"; do
      rvm use $MAJOR.$MINOR.$PATCH-float_warnings --patch patches/ruby/$MAJOR.$MINOR.$PATCH/float_warnings/01-bigdecimal_float_warning.patch --install
      ruby -e "require 'bigdecimal'; require 'bigdecimal/util'; BigDecimal.new(123.4, 2) && 7.1.to_d"
      rvm remove $MAJOR.$MINOR.$PATCH-float_warnings
    done
  done
  for MINOR in "2"; do
    for PATCH in "0" "1" "2" "3" "4" "5" "6" "7" "8"; do
      rvm use $MAJOR.$MINOR.$PATCH-float_warnings --patch patches/ruby/$MAJOR.$MINOR.$PATCH/float_warnings/01-bigdecimal_float_warning.patch --install
      ruby -e "require 'bigdecimal'; require 'bigdecimal/util'; BigDecimal.new(123.4, 2) && 7.1.to_d"
      rvm remove $MAJOR.$MINOR.$PATCH-float_warnings
    done
  done
  for MINOR in "3"; do
    for PATCH in "0" "1" "2" "3" "4" "5"; do
      rvm use $MAJOR.$MINOR.$PATCH-float_warnings --patch patches/ruby/$MAJOR.$MINOR.$PATCH/float_warnings/01-bigdecimal_float_warning.patch --install
      ruby -e "require 'bigdecimal'; require 'bigdecimal/util'; BigDecimal.new(123.4, 2) && 7.1.to_d"
      rvm remove $MAJOR.$MINOR.$PATCH-float_warnings
    done
  done
  for MINOR in "4"; do
    for PATCH in "0" "1" "2"; do
      rvm use $MAJOR.$MINOR.$PATCH-float_warnings --patch patches/ruby/$MAJOR.$MINOR.$PATCH/float_warnings/01-bigdecimal_float_warning.patch --install
      ruby -e "require 'bigdecimal'; require 'bigdecimal/util'; BigDecimal.new(123.4, 2) && 7.1.to_d"
      rvm remove $MAJOR.$MINOR.$PATCH-float_warnings
    done
  done
done

wteuber and others added 4 commits October 8, 2017 02:10
…2.2.[4 5 6 7 8], 2.3.[0 1 2 3 4 5] and 2.4.[0 1 2]
…ruby_versions

Add float_warnings patchsets for Ruby 2.0.0p64[5 7 8], 2.1.[8 9 10], 2.2.[4 5 6 7 8], 2.3.[0 1 2 3 4 5] and 2.4.[0 1 2]
@pkuczynski
Copy link
Member

Hi @knugie, could you shed some light on the source of this patches and what they supposed to fix?

@pkuczynski pkuczynski requested a review from skaes October 10, 2017 10:35
@knugie
Copy link
Contributor Author

knugie commented Oct 10, 2017

Hi @pkuczynski, of course. float_warnings is supposed to help Ruby developers identify unexpected behaviour when initializing BigDecimal with a Float. The patches add two warnings. One in bigdecimal C code BigDecimal.new and one in bigdecimal/util Ruby code Float#to_d.

fmi, see my closed rvm PRs: https://github.com/rvm/rvm/pulls?q=is%3Apr+author%3Aknugie+is%3Aclosed
and the original talk that made me create float_warnings: https://slides.com/wolfgangteuber/saving-money-with-ruby/fullscreen#/

Btw, it's totally unrelated to Railsexpressruby. I simply use @skaes' pattern to provide patchsets.
I hope that makes sense, happy to explain in more detail if needed.

@pkuczynski
Copy link
Member

Yeah, it makes perfect sense now. I was wondering if it's somehow connected with @skaes. I will merge it in, as it was merged before by @mpapis. This is not exactly what RVM intended to do, but I see a value in this feature...

@pkuczynski pkuczynski changed the title Add float_warnings patchsets for Ruby 2.0.0p64[5 7 8], 2.1.[8 9 10], 2.2.[4 5 6 7 8], 2.3.[0 1 2 3 4 5] and 2.4.[0 1 2] Add float_warnings patches for Ruby 2.0.0p64[5 7 8], 2.1.[8 9 10], 2.2.[4 5 6 7 8], 2.3.[0 1 2 3 4 5] and 2.4.[0 1 2] Oct 10, 2017
@pkuczynski pkuczynski merged commit c1ea491 into rvm:master Oct 10, 2017
@pkuczynski pkuczynski added this to the rvm-1.29.4 milestone Sep 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants