FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: workaround clang-3.4 ICE by ofrobots · Pull Request #8343 · nodejs/node · GitHub

/ node Public

deps: workaround clang-3.4 ICE - #8343

Merged
ofrobots merged 1 commit into
nodejs:masterfrom
ofrobots:workaround-for-clang-3.4
Aug 31, 2016
ConversationCommits (1)ChecksFiles changed
Merged

deps: workaround clang-3.4 ICE#8343
ofrobots merged 1 commit into
nodejs:masterfrom
ofrobots:workaround-for-clang-3.4

Conversation

ofrobots commented Aug 30, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps: v8

Description of change

Work around internal compiler error with clang-3.4.

R=@nodejs/v8
/cc @chrislea
Fixes: #8323

EDIT: CI: https://ci.nodejs.org/job/node-test-pull-request/3893/
EDIT: V8-CI: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/293/

nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Aug 30, 2016
ofrobots mentioned this pull request Aug 30, 2016
Closed

mscdex commented Aug 30, 2016

Copy link
Copy Markdown
Contributor

Is there already an upstream bug fix or at least an upstream issue for this that might be worth linking to also?

ofrobots commented Aug 30, 2016

Copy link
Copy Markdown
Contributor Author

I don't have an upstream bug fix link, but the issue seems to be fixed in clang-3.4.2. As per #8323 (comment), it is non-trivial to upgrade to clang-3.4.2 or newer on the wheezy & precise configurations.

ofrobots mentioned this pull request Aug 31, 2016
Closed
2 tasks

rvagg commented Aug 31, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Aside from objecting to the ugliness of C++, this LGTM

Green CI run @ https://ci.nodejs.org/job/node-test-commit/4862/

Most notably, clang 3.4.1 run @ https://ci.nodejs.org/job/node-test-commit-linux/4917/nodes=ubuntu1204-clang341-64/

ofrobots changed the base branch from v6.x to master August 31, 2016 21:03
ofrobots force-pushed the workaround-for-clang-3.4 branch from e926bbd to dc33d15 Compare August 31, 2016 21:06

ofrobots commented Aug 31, 2016

Copy link
Copy Markdown
Contributor Author

I have retargeted this PR to master (from v6.x) now that we have a (broken) CI builder for clang-3.4.

New CI: https://ci.nodejs.org/job/node-test-pull-request/3911/

rvagg commented Aug 31, 2016

Copy link
Copy Markdown
Member

CI is green (enough). Debian8 failure is Jenkins, I've updated and restarted that node. OSX failure is known flaky (#4629). clang 4.3.1 is 👍

lgtm still

deps: workaround clang-3.4 ICE
870faee 
PR-URL: nodejs#8343
Fixes: nodejs#8323
Reviewed-By: rvagg - Rod Vagg <rod@vagg.org>
ofrobots force-pushed the workaround-for-clang-3.4 branch from dc33d15 to 870faee Compare August 31, 2016 22:17
ofrobots merged commit 870faee into nodejs:master Aug 31, 2016

ofrobots commented Aug 31, 2016

Copy link
Copy Markdown
Contributor Author

Thanks landed as 870faee.

ofrobots deleted the workaround-for-clang-3.4 branch August 31, 2016 22:19
Fishrock123 mentioned this pull request Sep 6, 2016
Closed
addaleax mentioned this pull request Sep 8, 2016
Closed
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
deps: workaround clang-3.4 ICE
a6ba0a8 
PR-URL: nodejs#8343
Fixes: nodejs#8323
Reviewed-By: rvagg - Rod Vagg <rod@vagg.org>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
deps: workaround clang-3.4 ICE
c8bcf1b 
PR-URL: #8343
Fixes: #8323
Reviewed-By: rvagg - Rod Vagg <rod@vagg.org>
bnoordhuis mentioned this pull request Sep 12, 2016
Merged
2 tasks
ofrobots pushed a commit to ofrobots/node that referenced this pull request Sep 12, 2016
deps: cherry-pick workaround for clang-3.4 ICE
499e8b7 
Ref: nodejs#8343
Fixes: nodejs#8323
targos added a commit to targos/node that referenced this pull request Sep 22, 2016
deps: cherry-pick workaround for clang-3.4 ICE
91b4094 
Ref: nodejs#8343
Fixes: nodejs#8323

PR-URL: nodejs#8317
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
jasnell pushed a commit that referenced this pull request Sep 29, 2016
deps: cherry-pick workaround for clang-3.4 ICE
a88bb3a 
Ref: #8343
Fixes: #8323

PR-URL: #8317
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos added a commit to targos/node that referenced this pull request Dec 30, 2016
deps: cherry-pick workaround for clang-3.4 ICE
f5d459f 
Ref: nodejs#8343
Fixes: nodejs#8323

PR-URL: nodejs#8317
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos added a commit to targos/node that referenced this pull request Feb 7, 2017
deps: cherry-pick workaround for clang-3.4 ICE
a6715e5 
Fixes: nodejs#8323
Refs: nodejs#8343
Refs: nodejs#8317

PR-URL: nodejs#11029
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 14, 2017
deps: cherry-pick workaround for clang-3.4 ICE
7fea966 
Fixes: nodejs#8323
Refs: nodejs#8343
Refs: nodejs#8317

PR-URL: nodejs#11029
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
targos mentioned this pull request Mar 29, 2017
Closed
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

v6.5.0 fails to build using clang-3.4.

4 participants

Back | FazBrowse Home | New Git URL