Skip to content

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 9, 2021

Original commit message:

torque: workaround stod() limitations on Solaris

std::stod() on Solaris does not currently handle hex strings.
This commit provides a workaround based on strtol() until proper
stod() support is available.

This was encountered while updating Node.js to V8 8.8. For more
details see the following comment:

https://github.com/nodejs/node/pull/36139#issuecomment-740131942

Change-Id: I16ed80a817f6d9105e7153b10824b1fee8520432
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2692746
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73255}

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Mar 9, 2021
@targos
Copy link
Member

targos commented Mar 9, 2021

Please add Refs: https://github.com/v8/v8/commit/1648e050cade0e0803714a488ab0c8817d9eaf5c to the commit message.
I suppose we can ignore the embedder string update because it was already done in 19d9752

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 10, 2021

Original commit message:

    torque: workaround stod() limitations on Solaris

    std::stod() on Solaris does not currently handle hex strings.
    This commit provides a workaround based on strtol() until proper
    stod() support is available.

    This was encountered while updating Node.js to V8 8.8. For more
    details see the following comment:

    nodejs#36139 (comment)

    Change-Id: I16ed80a817f6d9105e7153b10824b1fee8520432
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2692746
    Reviewed-by: Michael Stanton <mvstanton@chromium.org>
    Commit-Queue: Michael Stanton <mvstanton@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#73255}

Refs: v8/v8@1648e05
PR-URL: nodejs#37664
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@cjihrig cjihrig merged commit 8a6d7a0 into nodejs:master Mar 11, 2021
@cjihrig cjihrig deleted the cherry branch March 11, 2021 02:30
danielleadams pushed a commit that referenced this pull request Mar 16, 2021
Original commit message:

    torque: workaround stod() limitations on Solaris

    std::stod() on Solaris does not currently handle hex strings.
    This commit provides a workaround based on strtol() until proper
    stod() support is available.

    This was encountered while updating Node.js to V8 8.8. For more
    details see the following comment:

    #36139 (comment)

    Change-Id: I16ed80a817f6d9105e7153b10824b1fee8520432
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2692746
    Reviewed-by: Michael Stanton <mvstanton@chromium.org>
    Commit-Queue: Michael Stanton <mvstanton@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#73255}

Refs: v8/v8@1648e05
PR-URL: #37664
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 16, 2021
Original commit message:

    torque: workaround stod() limitations on Solaris

    std::stod() on Solaris does not currently handle hex strings.
    This commit provides a workaround based on strtol() until proper
    stod() support is available.

    This was encountered while updating Node.js to V8 8.8. For more
    details see the following comment:

    #36139 (comment)

    Change-Id: I16ed80a817f6d9105e7153b10824b1fee8520432
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2692746
    Reviewed-by: Michael Stanton <mvstanton@chromium.org>
    Commit-Queue: Michael Stanton <mvstanton@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#73255}

Refs: v8/v8@1648e05
PR-URL: #37664
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants