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

Float class (backend) #443

Merged
merged 10 commits into from
Oct 6, 2017
Merged

Float class (backend) #443

merged 10 commits into from
Oct 6, 2017

Conversation

64kramsystem
Copy link
Member

@64kramsystem 64kramsystem commented Oct 3, 2017

Backend implementation of the Float class, without the frontend (parsing). At this stage, Floats are instantiated via String#to_f.

Two related additions are made to the codebase:

  • the Numeric interface, common to Float and Integer; currently it's only a lightweight, convenient, interface, not a full/visible Goby class
  • the static method toBooleanObject(), which replaces the boilerplate for converting Go boolean values to a Goby boolean Object

This PR closes the backend side of #379.

@ghost ghost assigned 64kramsystem Oct 3, 2017
@ghost ghost added the in progress label Oct 3, 2017
@64kramsystem 64kramsystem changed the title WIP: Float class WIP: Float class (backend) Oct 3, 2017
@codecov
Copy link

codecov bot commented Oct 3, 2017

Codecov Report

Merging #443 into master will increase coverage by 0.34%.
The diff coverage is 95.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #443      +/-   ##
==========================================
+ Coverage   82.55%   82.89%   +0.34%     
==========================================
  Files          43       44       +1     
  Lines        7750     7946     +196     
==========================================
+ Hits         6398     6587     +189     
- Misses       1133     1141       +8     
+ Partials      219      218       -1
Impacted Files Coverage Δ
vm/string.go 96.56% <100%> (+0.06%) ⬆️
vm/boolean.go 95.31% <100%> (+0.39%) ⬆️
vm/vm.go 86.36% <100%> (+0.05%) ⬆️
vm/integer.go 77.77% <100%> (+2.36%) ⬆️
vm/float.go 91.32% <91.32%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87c7439...cf37789. Read the comment docs.

@64kramsystem 64kramsystem force-pushed the sm-float branch 2 times, most recently from 57934b0 to ead2771 Compare October 3, 2017 20:51
@64kramsystem
Copy link
Member Author

Reference: Ruby strategy for cross-data type operations:

/*
 * call-seq:
 *   float + other  ->  float
 *
 * Returns a new Float which is the sum of +float+ and +other+.
 */

static VALUE
flo_plus(VALUE x, VALUE y)
{
    if (RB_TYPE_P(y, T_FIXNUM)) {
  return DBL2NUM(RFLOAT_VALUE(x) + (double)FIX2LONG(y));
    }
    else if (RB_TYPE_P(y, T_BIGNUM)) {
  return DBL2NUM(RFLOAT_VALUE(x) + rb_big2dbl(y));
    }
    else if (RB_TYPE_P(y, T_FLOAT)) {
  return DBL2NUM(RFLOAT_VALUE(x) + RFLOAT_VALUE(y));
    }
    else {
  return rb_num_coerce_bin(x, y, '+');
    }
}

And (see https://silverhammermba.github.io/emberb/c/):

There are a host of “NUM” macros that try to be more duck-typish about things. These will convert their C types to whatever Ruby Numeric subclass seems appropriate:

...
DBL2NUM() for double

@64kramsystem
Copy link
Member Author

64kramsystem commented Oct 3, 2017

@st0012 should the equivalent of Integer#to_intNN be implemented for Float? For example, Float#to_float32?

The implementation is easy, I'm wondering what's the rationale (purpose), since there is nothing equivalent in Ruby.

@64kramsystem 64kramsystem force-pushed the sm-float branch 8 times, most recently from c9d9f51 to 8df1d37 Compare October 3, 2017 21:55
@st0012
Copy link
Member

st0012 commented Oct 4, 2017

@saveriomiroddi The purpose for those methods is for calling Go functions. For current Integer class, those methods just change integer object's flag, and those flags only will be used when the integer is passed as Go function's parameters.

But I think I'll change the implementation of this by letting methods like to_int8 return a GoObject that really contains the int8 type integer. Because mutation is bad.

@64kramsystem
Copy link
Member Author

@st0012 ready for review.

@64kramsystem 64kramsystem changed the title WIP: Float class (backend) Float class (backend) Oct 5, 2017
{`25 / 5`, 5},
{`1 / 1 + 1`, 2},
{`0 / (1 + 1000)`, 0},
{`5 ** (3 * 2) + 21`, 15646},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saveriomiroddi I want to keep these combined arithmetic operations test cases. Can you create another test function for these kind of tests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Restored!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 😎

@st0012
Copy link
Member

st0012 commented Oct 6, 2017

BTW thanks for reorganizing our integer tests at the same time 👍

@st0012 st0012 merged commit 791f8ae into goby-lang:master Oct 6, 2017
@ghost ghost removed the in progress label Oct 6, 2017
@64kramsystem 64kramsystem deleted the sm-float branch October 6, 2017 15:19
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.

2 participants