Skip to content
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

Support LLVM 5.0 #4821

Merged
merged 2 commits into from
Sep 8, 2017
Merged

Conversation

ysbaddaden
Copy link
Contributor

Adds support for LLVM 5.0 (rc2). They didn't introduce much breaking changes, for once.

Warning: builds on #4396 (drop LLVM 3.5 and 3.6 support) which should be merged beforehand, or merged directly with this pull request.

The supported LLVM versions are 3.8, 3.9 and 4.0.
@ysbaddaden
Copy link
Contributor Author

@akzhan
Copy link
Contributor

akzhan commented Sep 3, 2017

Just want to note that we must drop support for buggy LLVM <3.9. So please rebuild omnibuses and get LLVM 5 compatibility.

@sdogruyol
Copy link
Member

sdogruyol commented Sep 7, 2017

LLVM 5.0 is just released http://releases.llvm.org/5.0.0/docs/ReleaseNotes.html 👍
LGTM?

@asterite asterite mentioned this pull request Sep 8, 2017
@asterite
Copy link
Member

asterite commented Sep 8, 2017

CI failed. There's some bug in the LLVM bindings that I can't find... an LLVMContext is disposed but apparently the debug metadata associated with it was already disposed... but I don't know where...

@asterite
Copy link
Member

asterite commented Sep 8, 2017

In any case it's something that I think should only happen in compiler specs, and it has nothing to do with the LLVM version, so we can tackle that later at some point.

Should we merge this? All specs are passing, which is really good because they broke once brew install llvm changes to LLVM 5.0 :-)

@ysbaddaden
Copy link
Contributor Author

I rebuilt on Circle and it passed this time. Random failures...

I'll be happy to merge. Omnibus should already be using LLVM 3.8 now, and if need be it may revert thus patch to reenable LLVM 3.5, but I doubt this is needed.

@RX14 RX14 merged commit 3d48a96 into crystal-lang:master Sep 8, 2017
@RX14 RX14 added this to the Next milestone Sep 8, 2017
@Sija
Copy link
Contributor

Sija commented Sep 9, 2017

@RX14 what about #4396 mentioned in PR's description?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants