mmarchini commented 4 years ago

Type of String::Length changed from SMI to int32. This commit changes String::Length to use the new type. String::Length now returns a CheckedType, and places calling String::Length will check the type using .Check instead of Error::Fail, making the code more resilient in case the String length can't be loaded properly.

codecov-io commented 4 years ago

Codecov Report

Merging #302 into master will increase coverage by 0.4%. The diff coverage is 58.06%.

@@           Coverage Diff           @@
##           master   #302     +/-   ##
=======================================
+ Coverage    78.6%    79%   +0.4%     
=======================================
  Files          33     33             
  Lines        4225   4244     +19     
=======================================
+ Hits         3321   3353     +32     
+ Misses        904    891     -13

Impacted Files	Coverage Δ
src/llv8.h	`83.33% <ø> (+2.38%)`	:arrow_up:
src/llv8-constants.h	`100% <ø> (+1.51%)`	:arrow_up:
src/llv8.cc	`72.81% <0%> (+1.69%)`	:arrow_up:
src/llv8-constants.cc	`83.06% <50%> (+0.54%)`	:arrow_up:
src/llv8-inl.h	`92.91% <80%> (+0.33%)`	: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 2c4c99c...a3c7f2b. Read the comment docs.

mmarchini commented 4 years ago

For posterity, the SMI to int32 change happened here: https://chromium-review.googlesource.com/c/v8/v8/+/1224432/

I'm not sure this pull request fixes all the problems though. I'm still seeing a lot of frames not loading with v8 bt...

mmarchini commented 4 years ago

cc @nodejs/llnode

mmarchini commented 4 years ago

Landed in a79a008

nodejs / llnode

src: fix String::Length for Node.js v12.3.0 #302

Codecov Report