-
Notifications
You must be signed in to change notification settings - Fork 171
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
Support Formal Front-End of Float Type Data #470
Conversation
if ok && p.peekTokenIs(token.Int) { | ||
// When both receiver & caller are integer => Float | ||
return p.parseFloatLiteral(receiver) | ||
} else { |
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.
if block ends with a return statement, so drop this else and outdent its block
compiler/ast/expressions.go
Outdated
} | ||
|
||
func (il *FloatLiteral) expressionNode() {} | ||
func (il *FloatLiteral) TokenLiteral() string { |
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.
exported method FloatLiteral.TokenLiteral should have comment or be unexported
compiler/ast/expressions.go
Outdated
@@ -19,6 +19,19 @@ func (il *IntegerLiteral) String() string { | |||
return il.Token.Literal | |||
} | |||
|
|||
type FloatLiteral struct { |
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.
exported type FloatLiteral should have comment or be unexported
if ok && p.peekTokenIs(token.Int) { | ||
// When both receiver & caller are integer => Float | ||
return p.parseFloatLiteral(receiver) | ||
} else { |
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.
if block ends with a return statement, so drop this else and outdent its block
compiler/ast/expressions.go
Outdated
} | ||
|
||
func (il *FloatLiteral) expressionNode() {} | ||
func (il *FloatLiteral) TokenLiteral() string { |
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.
exported method FloatLiteral.TokenLiteral should have comment or be unexported
compiler/ast/expressions.go
Outdated
@@ -19,6 +19,19 @@ func (il *IntegerLiteral) String() string { | |||
return il.Token.Literal | |||
} | |||
|
|||
type FloatLiteral struct { |
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.
exported type FloatLiteral should have comment or be unexported
if ok && p.peekTokenIs(token.Int) { | ||
// When both receiver & caller are integer => Float | ||
return p.parseFloatLiteral(receiver) | ||
} else { |
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.
if block ends with a return statement, so drop this else and outdent its block
compiler/ast/expressions.go
Outdated
} | ||
|
||
func (il *FloatLiteral) expressionNode() {} | ||
func (il *FloatLiteral) TokenLiteral() string { |
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.
exported method FloatLiteral.TokenLiteral should have comment or be unexported
compiler/ast/expressions.go
Outdated
@@ -19,6 +19,19 @@ func (il *IntegerLiteral) String() string { | |||
return il.Token.Literal | |||
} | |||
|
|||
type FloatLiteral struct { |
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.
exported type FloatLiteral should have comment or be unexported
@st0012 Can you hint me on writing tests about |
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.
I think you just need to make VM's tests all be passed for now
vm/float_test.go
Outdated
@@ -200,8 +200,8 @@ func TestFloatConversions(t *testing.T) { | |||
input string | |||
expected interface{} | |||
}{ | |||
{`'100.3'.to_f.to_i`, 100}, | |||
{`'100.3'.to_f.to_s`, "100.3"}, | |||
{`(100.3).to_i`, 100}, |
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.
We should also support (or raise parsing error) when there's no parentheses around the float.
@Maxwell-Alexius We'll have an large reconstruction for our compiler's tests, so don't worry about adding parser tests now. |
For reference, programming languages generally support also the exponential format:
I think this should be either implemented in this PR, or a separate issue should be created, for tracking purposes. |
@saveriomiroddi Thanks for reminding, I'll create another issue for this format. |
a26852c
to
d968d3f
Compare
Codecov Report
@@ Coverage Diff @@
## master #470 +/- ##
==========================================
- Coverage 83.6% 83.29% -0.31%
==========================================
Files 53 53
Lines 9325 9369 +44
==========================================
+ Hits 7796 7804 +8
- Misses 1289 1320 +31
- Partials 240 245 +5
Continue to review full report at Codecov.
|
e428340
to
5991cbe
Compare
e0c381c
to
c365987
Compare
4d9bb06
to
e935aec
Compare
e935aec
to
19972aa
Compare
@st0012 Currently there is a small problem -- cases below will show the correct result (3.14159265358979).to_d.to_s # => "3.14159265358979"
(-273.150000000).to_d.to_s # => "-273.15"
# and without the round bracket
3.14159265358979.to_d.to_s # => "3.14159265358979" However, there is a case when a negative valued float type number cannot call method without round bracket which I've labeled with a TODO -273.150000000.to_d.to_s # ERROR |
3.14159265358979.to_d.to_s`, | ||
"3.14159265358979"}, | ||
// TODO: Able to parse negative float value and call method without parentheses | ||
//{` |
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.
Here is the small problem which is described in PR
Another thing to address is that, currently this PR only support the basic format of Float API (and because this PR is big enough), so to implement the special format of float, e.g. the exponential format as described in issue #471, welcome to contribute another PR |
@saveriomiroddi Do you want to take a look before I merge this? |
_, ok := receiver.(*ast.IntegerLiteral) | ||
|
||
// When both receiver & caller are integer => Float | ||
if ok && p.peekTokenIs(token.Int) { |
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.
Based on this code, expresions with spaces on the sides of the dot will be parsed as floats (eg. 2 . 3
).
If this is intended (other scripting languages forbid it), I think it should be documented with a test case.
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.
Actually currently Goby doesn't recognize white space, so it's not just float has this issue, like:
[] . length #=> 0
But this shouldn't be too hard to solve I think, already have a rough idea. But that won't be included in this PR.
@@ -438,7 +438,7 @@ func (f *FloatObject) numericComparison(t *thread, rightObject Object, operation | |||
// toString returns the object's value as the string format, in non | |||
// exponential format (straight number, without exponent `E<exp>`). | |||
func (f *FloatObject) toString() string { | |||
return strconv.FormatFloat(f.value, 'f', -1, 64) | |||
return strconv.FormatFloat(f.value, 'f', -1, 64) // fmt.Sprintf("%f", f.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.
Is the comment intended?
Currently using the Go
float64
datatype to support for Goby's float type dataSmall issue to address - cases below will show the correct result
However, there is a case when a negative valued float type number cannot call method without round bracket which I've labeled with a TODO
See #526
Another thing to address is that, currently this PR only support the basic format of Float API (and because this PR is big enough), so to implement the special format of float, e.g. the exponential format as described in issue #471, welcome to contribute another PR