boa icon indicating copy to clipboard operation
boa copied to clipboard

JsValue from primitives

Open lastmjs opened this issue 2 years ago • 2 comments

This Pull Request fixes/closes #1994 and #1991 and depends on https://github.com/boa-dev/boa/pull/1971.

It changes the following:

  • Adds many primitive conversions, From<T> for JsValue where T are integers
  • Makes some opinions about i128, u128, i64, and u64 when converting to JsValue, always converting to BigInt

lastmjs avatar Mar 31 '22 13:03 lastmjs

This change reduces our conformance by a very large amount. The reason is the conversion for i64, IIRC.

The range of an integer index in javascript is, surprisingly, [0 to 2^53), which is the max integer that is representable by an f64 without loss of precision. When we parse an integer, we use i32 to store it for performance reasons, and fallback to f64 in case the integer exceeds the max value of an i32, because converting from f64 to u64 is faster than converting from JsBigInt to u64. However, eagerly converting it to JsBigInt automatically makes it unusable as an integer index, and it's instead converted to a string key. This unfortunately breaks indexing for big integer values.

jedel1043 avatar Mar 31 '22 15:03 jedel1043

Codecov Report

Merging #1993 (39e2799) into main (6baf455) will decrease coverage by 0.03%. The diff coverage is 3.33%.

@@            Coverage Diff             @@
##             main    #1993      +/-   ##
==========================================
- Coverage   45.89%   45.85%   -0.04%     
==========================================
  Files         206      206              
  Lines       17150    17170      +20     
==========================================
+ Hits         7871     7874       +3     
- Misses       9279     9296      +17     
Impacted Files Coverage Δ
boa_engine/src/bigint.rs 37.95% <0.00%> (-1.15%) :arrow_down:
boa_engine/src/value/mod.rs 51.09% <0.00%> (-0.76%) :arrow_down:
boa_engine/src/value/conversions.rs 19.64% <4.54%> (-5.36%) :arrow_down:
boa_engine/src/builtins/string/mod.rs 62.74% <0.00%> (+0.21%) :arrow_up:
...ine/src/syntax/parser/expression/assignment/mod.rs 30.84% <0.00%> (+1.86%) :arrow_up:

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 6baf455...39e2799. Read the comment docs.

codecov[bot] avatar Apr 03 '22 00:04 codecov[bot]

I'm going to close this pull request in response to the comments.

lastmjs avatar Dec 16 '22 20:12 lastmjs