-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fixes for NPM test breaks. #4659
Conversation
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.
Looks generally fine to me. If possible, I think I'd prefer to store the new.target
value in a separate field so it's not confusing down the line but I can accept this as-is.
lib/Runtime/Debug/TTEvents.cpp
Outdated
|
||
if (args.HasNewTarget()) | ||
{ | ||
callEvt->ArgArray[args.Info.Count + 1] = static_cast<TTDVar>(args.GetNewTarget()); |
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.
Would it make sense to allocate one more pointer in callEvt
to handle the new.target
(callEvt->newTargetPointer
?) instead of tacking it on at the end? This is our data structure so we don't really need to sneakily add an extra arg here and it might be confusing why the argument count doesn't match.
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.
@mrkmarron can you have a look at the change I made for this?
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.
lgtm
lgtm |
Merge pull request #4659 from MSLaguana:fixTTD Some node-chakracore tests were throwing asserts when run with TTD.
Merge pull request #4659 from MSLaguana:fixTTD Some node-chakracore tests were throwing asserts when run with TTD.
Some node-chakracore tests were throwing asserts when run with TTD.