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

crystal --version reports LLVM_VERSION #4095

Merged
merged 1 commit into from
Mar 2, 2017
Merged

Conversation

matiasgarciaisaia
Copy link
Member

This PR makes crystal --version report the LLVM version the compiler is using. It also adds a Crystal::LLVM_VERSION constant that user code can access, analogous to Crystal::VERSION.

Hopefully, it will help troubleshoot compiler bugs that are related to changes in LLVM versions.

else
"Crystal #{version} (#{date})"
"Crystal #{version} (#{date}) LLVM #{llvm_version}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put it on a new line?

Crystal 0.21.0 (2017-02-20)
LLVM 4.0

My thinking is that it would make it easier to report other info in the future (stick each item on a new line).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that - a little bit inspired in docker version - but I couldn't come with any other thing I wanted to see reported there.

I thought maybe a Crystal program would specify this versions in it's own --version command - say, running my-tool --version would report both my-tool's, Crystal's & LLVM's versions, but @asterite somewhat convinced me you shouldn't care about LLVM version in client code (btw, do you agree with this?).

I'm open to change my mind, anyway :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one line is fine for now. Maybe we can discuss the general output of crystal version in another issue :-)

@asterite asterite merged commit 1ec9806 into master Mar 2, 2017
@asterite asterite added this to the Next milestone Mar 2, 2017
@asterite asterite deleted the feature/llvm-version branch April 27, 2018 21:17
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.

3 participants