Fix #331 "We only get `Const::Num`, never `Const::Int`" #338

HalidOdat · 2020-04-19T17:49:34Z

This will close #331 and unblock #328

github-actions · 2020-04-19T18:01:40Z

Benchmark for `c815def`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	324.5±7.36µs	328.9±7.69µs	99%
Expression (Lexer)	1806.2±67.10ns	1995.4±88.28ns	89.99999999999999%
Expression (Parser)	4.4±0.31µs	4.8±0.11µs	93%
Fibonacci (Execution)	3.5±0.09ms	3.5±0.07ms	100%
For loop (Execution)	332.3±12.31µs	333.6±5.64µs	100%
For loop (Lexer)	4.8±0.20µs	5.0±0.46µs	97%
For loop (Parser)	12.0±0.56µs	11.9±0.36µs	101%
Hello World (Lexer)	858.2±24.98ns	849.5±34.10ns	101%
Hello World (Parser)	1959.4±125.09ns	1970.0±73.00ns	99%
Symbols (Execution)	348.0±31.15µs	353.3±14.36µs	98%
	undefined	undefined	100%

jasonwilliams · 2020-04-21T16:57:15Z

Thanks, will try and get round to looking at this soon

Razican

It's looking pretty good. Please check my comments :)

boa/src/builtins/function/tests.rs

boa/src/syntax/lexer/mod.rs

boa/src/syntax/ast/token.rs

github-actions · 2020-04-21T18:16:12Z

Benchmark for `706f34f`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	326.9±11.93µs	341.7±40.48µs	95%
Expression (Lexer)	1753.1±73.56ns	1996.5±70.01ns	86.00000000000001%
Expression (Parser)	4.3±0.12µs	4.7±0.24µs	90.99999999999999%
Fibonacci (Execution)	3.6±0.11ms	3.4±0.08ms	104%
For loop (Execution)	358.3±49.34µs	344.3±8.60µs	104%
For loop (Lexer)	4.9±0.08µs	4.8±0.08µs	102%
For loop (Parser)	11.8±0.30µs	12.0±0.40µs	99%
Hello World (Lexer)	853.9±26.62ns	847.8±21.76ns	101%
Hello World (Parser)	1932.4±59.40ns	1959.2±112.19ns	99%
Symbols (Execution)	356.2±12.32µs	353.5±5.75µs	101%
	undefined	undefined	100%

github-actions · 2020-04-21T18:16:15Z

Benchmark for `1b2481c`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	347.7±19.19µs	348.4±17.16µs	100%
Expression (Lexer)	1855.6±106.72ns	2.2±0.14µs	84.00000000000001%
Expression (Parser)	4.4±0.29µs	4.8±0.27µs	90.99999999999999%
Fibonacci (Execution)	3.9±0.16ms	3.8±0.12ms	102%
For loop (Execution)	363.2±15.82µs	357.4±22.51µs	102%
For loop (Lexer)	5.1±0.33µs	5.0±0.33µs	102%
For loop (Parser)	12.9±0.61µs	12.8±0.58µs	100%
Hello World (Lexer)	903.7±51.82ns	901.3±45.13ns	100%
Hello World (Parser)	2.0±0.10µs	2.1±0.20µs	99%
Symbols (Execution)	374.8±20.16µs	376.7±14.89µs	100%
	undefined	undefined	100%

github-actions · 2020-04-21T18:16:34Z

Benchmark for `125adbb`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	324.8±18.59µs	338.1±20.96µs	96%
Expression (Lexer)	1754.9±94.22ns	2.1±0.11µs	81%
Expression (Parser)	4.3±0.31µs	4.7±0.24µs	89.99999999999999%
Fibonacci (Execution)	3.7±0.17ms	3.7±0.18ms	100%
For loop (Execution)	382.1±9.44µs	346.4±22.49µs	110.00000000000001%
For loop (Lexer)	4.7±0.29µs	5.0±0.39µs	93%
For loop (Parser)	12.2±0.57µs	12.1±0.68µs	101%
Hello World (Lexer)	858.2±54.77ns	852.0±42.59ns	101%
Hello World (Parser)	1965.8±112.45ns	1894.2±125.23ns	104%
Symbols (Execution)	355.0±16.43µs	353.8±15.08µs	100%
	undefined	undefined	100%

HalidOdat · 2020-04-23T17:01:33Z

I reworked the number lexing and fixed some things:

Added some documentation to lexer and token
Fixed a bug were it did not accept 09.5e2.
Made it easy to add strict mode for number literal (in strict mode 034 and 098 is an error).
Made it easy to add BigInt support (including other base BigInts).
Added Value::is_integer to check if a number is an integer.
Renamed ValueData::Number(f64) to ValueData::Rational(f64)
Fixed position bug related to numeric tokens.
Added some tests for edge case with implicit octal and hexadecimals followed by a dot
Split numerical lexing to a different function.
Removed read_integer_in_base.

This unblocks #328

I think now it's ready to review. :)

github-actions · 2020-04-23T17:35:49Z

Benchmark for `02a7c4a`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	346.5±22.77µs	360.0±21.00µs	96%
Expression (Lexer)	1993.3±105.40ns	2.2±0.08µs	89.99999999999999%
Expression (Parser)	4.8±0.19µs	5.1±0.18µs	94%
Fibonacci (Execution)	3.8±0.19ms	3.8±0.11ms	99%
For loop (Execution)	376.3±13.48µs	370.0±12.69µs	102%
For loop (Lexer)	5.4±0.32µs	5.5±0.19µs	97%
For loop (Parser)	14.0±0.78µs	14.0±0.62µs	100%
Hello World (Lexer)	1003.2±45.57ns	965.0±52.16ns	104%
Hello World (Parser)	2.2±0.14µs	2.2±0.08µs	100%
Symbols (Execution)	381.7±17.20µs	394.9±20.83µs	97%
	undefined	undefined	100%

Razican

This looks very good! I think we will see performance improvements in the execution too. Check my comments, specially about using Rational or Racional.

boa/src/builtins/value/mod.rs

boa/src/syntax/lexer/tests.rs

boa/src/syntax/ast/token.rs

boa/src/syntax/lexer/tests.rs

github-actions · 2020-04-24T13:40:30Z

Benchmark for `958dd8f`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	357.0±15.26µs	355.8±10.67µs	100%
Expression (Lexer)	2.1±0.12µs	2.3±0.11µs	92%
Expression (Parser)	4.8±0.25µs	5.0±0.19µs	95%
Fibonacci (Execution)	3.9±0.10ms	3.9±0.11ms	100%
For loop (Execution)	369.2±12.38µs	381.2±12.58µs	97%
For loop (Lexer)	5.5±0.18µs	5.5±0.35µs	100%
For loop (Parser)	14.4±0.88µs	14.6±1.00µs	98%
Hello World (Lexer)	1013.0±38.76ns	989.0±46.43ns	102%
Hello World (Parser)	2.3±0.14µs	2.3±0.14µs	99%
Symbols (Execution)	380.0±10.14µs	386.1±13.57µs	98%
	undefined	undefined	100%

github-actions · 2020-04-24T13:46:43Z

Benchmark for `6f8c46c`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	348.8±14.22µs	339.1±15.49µs	103%
Expression (Lexer)	1997.1±96.14ns	2.1±0.14µs	94%
Expression (Parser)	4.6±0.20µs	4.8±0.30µs	97%
Fibonacci (Execution)	3.7±0.17ms	3.7±0.18ms	102%
For loop (Execution)	351.8±23.20µs	347.6±15.00µs	101%
For loop (Lexer)	5.0±0.28µs	5.2±0.27µs	97%
For loop (Parser)	13.5±0.50µs	13.6±0.93µs	99%
Hello World (Lexer)	946.7±46.43ns	951.4±60.49ns	100%
Hello World (Parser)	2.1±0.11µs	2.1±0.12µs	101%
Symbols (Execution)	359.4±27.02µs	351.9±23.66µs	102%
	undefined	undefined	100%

HalidOdat · 2020-04-24T14:28:04Z

boa/src/syntax/lexer/mod.rs

+        // TODO: Setup strict mode.
+        let strict_mode = false;


Currently I have strict_mode set to always be false. In strict mode decimals with leading zeros and implicit octal are not allowed.

I seems like the parser should take in a lexer, because 'use strict'; it only applies to function or program scope and the lexer does not know what a scope is.

Yep, this is going to be interesting to see once we have the lexer as an iterator tied to the cursor.

Razican

This is good to go from my side :)

github-actions · 2020-04-27T23:40:49Z

Benchmark for `50eb93f`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	370.9±17.89µs	370.6±18.17µs	100%
Expression (Lexer)	2.0±0.27µs	2.1±0.11µs	97%
Expression (Parser)	4.7±0.27µs	4.9±0.20µs	95%
Fibonacci (Execution)	4.0±0.13ms	3.8±0.13ms	105%
For loop (Execution)	413.0±19.46µs	395.5±21.87µs	104%
For loop (Lexer)	5.3±0.51µs	5.4±0.31µs	98%
For loop (Parser)	13.5±0.89µs	13.5±0.48µs	100%
Hello World (Lexer)	955.5±39.03ns	1014.7±45.27ns	94%
Hello World (Parser)	2.2±0.11µs	2.2±0.12µs	97%
Symbols (Execution)	403.0±19.43µs	410.9±58.49µs	98%
	undefined	undefined	100%

Razican · 2020-04-28T11:59:08Z

I would wait for #304 to land before this.

HalidOdat · 2020-04-28T18:20:06Z

The parser is looking amazing! I will rebase this today!

github-actions · 2020-04-28T21:27:44Z

Benchmark for `779dd63`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	333.6±12.51µs	331.3±11.55µs	101%
Expression (Lexer)	1713.4±75.59ns	1860.1±67.02ns	90.99999999999999%
Expression (Parser)	4.5±0.18µs	4.6±0.25µs	97%
Fibonacci (Execution)	3.2±0.13ms	3.2±0.11ms	100%
For loop (Execution)	344.4±16.82µs	346.4±13.37µs	99%
For loop (Lexer)	4.6±0.21µs	4.7±0.22µs	99%
For loop (Parser)	12.4±0.51µs	12.6±0.62µs	98%
Hello World (Lexer)	822.0±30.67ns	827.2±32.52ns	99%
Hello World (Parser)	2.0±0.06µs	2.0±0.07µs	99%
Symbols (Execution)	357.1±13.73µs	363.3±17.71µs	98%
	undefined	undefined	100%

jasonwilliams

Looks good to me too!
Checks are failing though

github-actions · 2020-04-28T23:31:16Z

Benchmark for `3768535`

Click to view benchmark

Test	PR Benchmark	Master Benchmark	%
Create Realm	362.6±23.13µs	356.3±16.71µs	102%
Expression (Lexer)	1793.0±111.03ns	2.1±0.11µs	83%
Expression (Parser)	4.9±0.21µs	5.0±0.56µs	96%
Fibonacci (Execution)	3.5±0.14ms	3.3±0.18ms	106%
For loop (Execution)	371.1±18.74µs	375.4±24.77µs	99%
For loop (Lexer)	5.0±0.26µs	5.3±0.21µs	93%
For loop (Parser)	13.4±0.93µs	13.7±0.90µs	98%
Hello World (Lexer)	892.7±37.51ns	951.8±99.60ns	93%
Hello World (Parser)	2.2±0.23µs	2.3±0.15µs	98%
Symbols (Execution)	393.8±11.68µs	390.7±22.63µs	101%
	undefined	undefined	100%

HalidOdat added bug Something isn't working lexer Issues surrounding the lexer labels Apr 19, 2020

HalidOdat added this to the v0.8.0 milestone Apr 19, 2020

HalidOdat marked this pull request as ready for review April 19, 2020 17:54

HalidOdat changed the title ~~Fix #331~~ Fix #331 "We only get Const::Num, never `Const::Int" Apr 19, 2020

HalidOdat changed the title ~~Fix #331 "We only get Const::Num, never `Const::Int"~~ Fix #331 "We only get Const::Num, never Const::Int" Apr 19, 2020

HalidOdat changed the title ~~Fix #331 "We only get Const::Num, never Const::Int"~~ Fix #331 "We only get Const::Num, never Const::Int" Apr 19, 2020

HalidOdat changed the title ~~Fix #331 "We only get Const::Num, never Const::Int"~~ Fix #331 "We only get Const::Num, never Const::Int " Apr 19, 2020

Razican approved these changes Apr 21, 2020

View reviewed changes

HalidOdat marked this pull request as draft April 21, 2020 18:31

HalidOdat marked this pull request as ready for review April 23, 2020 17:01

HalidOdat requested review from Razican and jasonwilliams April 23, 2020 17:37

Razican reviewed Apr 24, 2020

View reviewed changes

boa/src/builtins/value/mod.rs Outdated Show resolved Hide resolved

boa/src/syntax/lexer/tests.rs Outdated Show resolved Hide resolved

boa/src/syntax/ast/token.rs Outdated Show resolved Hide resolved

boa/src/syntax/lexer/tests.rs Outdated Show resolved Hide resolved

HalidOdat requested a review from Razican April 24, 2020 13:49

HalidOdat commented Apr 24, 2020

View reviewed changes

Razican approved these changes Apr 24, 2020

View reviewed changes

HalidOdat force-pushed the fix-331 branch from d23c657 to 60794a0 Compare April 27, 2020 23:25

HalidOdat force-pushed the fix-331 branch from 60794a0 to e6265d7 Compare April 28, 2020 21:13

jasonwilliams approved these changes Apr 28, 2020

View reviewed changes

fix issue 331

63ef77b

HalidOdat force-pushed the fix-331 branch from e6265d7 to 63ef77b Compare April 28, 2020 23:16

HalidOdat changed the title ~~Fix #331 "We only get Const::Num, never Const::Int "~~ Fix #331 "We only get Const::Num, never Const::Int" Apr 28, 2020

HalidOdat merged commit 84b4da5 into master Apr 28, 2020

HalidOdat deleted the fix-331 branch April 28, 2020 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #331 "We only get `Const::Num`, never `Const::Int`" #338

Fix #331 "We only get `Const::Num`, never `Const::Int`" #338

HalidOdat commented Apr 19, 2020

github-actions bot commented Apr 19, 2020

jasonwilliams commented Apr 21, 2020

Razican left a comment

github-actions bot commented Apr 21, 2020

github-actions bot commented Apr 21, 2020

github-actions bot commented Apr 21, 2020

HalidOdat commented Apr 23, 2020 •

edited

Loading

github-actions bot commented Apr 23, 2020

Razican left a comment

github-actions bot commented Apr 24, 2020

github-actions bot commented Apr 24, 2020

HalidOdat Apr 24, 2020

Razican Apr 24, 2020

Razican left a comment

github-actions bot commented Apr 27, 2020

Razican commented Apr 28, 2020

HalidOdat commented Apr 28, 2020 •

edited

Loading

github-actions bot commented Apr 28, 2020

jasonwilliams left a comment •

edited

Loading

github-actions bot commented Apr 28, 2020

Fix #331 "We only get Const::Num, never Const::Int" #338

Fix #331 "We only get Const::Num, never Const::Int" #338

Conversation

HalidOdat commented Apr 19, 2020

github-actions bot commented Apr 19, 2020

Benchmark for c815def

jasonwilliams commented Apr 21, 2020

Razican left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 21, 2020

Benchmark for 706f34f

github-actions bot commented Apr 21, 2020

Benchmark for 1b2481c

github-actions bot commented Apr 21, 2020

Benchmark for 125adbb

HalidOdat commented Apr 23, 2020 • edited Loading

github-actions bot commented Apr 23, 2020

Benchmark for 02a7c4a

Razican left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 24, 2020

Benchmark for 958dd8f

github-actions bot commented Apr 24, 2020

Benchmark for 6f8c46c

HalidOdat Apr 24, 2020

Choose a reason for hiding this comment

Razican Apr 24, 2020

Choose a reason for hiding this comment

Razican left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 27, 2020

Benchmark for 50eb93f

Razican commented Apr 28, 2020

HalidOdat commented Apr 28, 2020 • edited Loading

github-actions bot commented Apr 28, 2020

Benchmark for 779dd63

jasonwilliams left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 28, 2020

Benchmark for 3768535

Fix #331 "We only get `Const::Num`, never `Const::Int`" #338

Fix #331 "We only get `Const::Num`, never `Const::Int`" #338

Benchmark for `c815def`

Benchmark for `706f34f`

Benchmark for `1b2481c`

Benchmark for `125adbb`

HalidOdat commented Apr 23, 2020 •

edited

Loading

Benchmark for `02a7c4a`

Benchmark for `958dd8f`

Benchmark for `6f8c46c`

Benchmark for `50eb93f`

HalidOdat commented Apr 28, 2020 •

edited

Loading

Benchmark for `779dd63`

jasonwilliams left a comment •

edited

Loading

Benchmark for `3768535`