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

Hide API removed in LLVM 17 #53

Closed
wants to merge 3 commits into from
Closed

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Sep 4, 2023

For simplicity, I've kept it all around for users that want to wait on moving to LLVM 17 fully, and just hidden it with build tags.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

I would like to keep the API and the tests the same (as much as possible) between LLVM versions.

Can you:

  • Remove APIs that were removed in LLVM (not just hide them with a build tag), for example the ConstSelect function.
  • Keep the tests the same, for example by removing the removed passes in executionengine_test.go.

@aykevl
Copy link
Member

aykevl commented Sep 18, 2023

For simplicity, I've kept it all around for users that want to wait on moving to LLVM 17 fully, and just hidden it with build tags.

In my opinion, people that are not ready to update to LLVM 17 should use an older version of this package that does include those older APIs.
(I'm saying that because the main consumer of go-llvm is TinyGo, and in TinyGo I am not going to use such older APIs when updating the LLVM version unless there is no other way to do something. If there are other users of this package that work differently, this can be reconsidered of course).

@aykevl
Copy link
Member

aykevl commented Sep 23, 2023

I have merged an alternative to this in #55 so closing. The APIs are now removed entirely, instead of hiding them with a build tag.

@aykevl aykevl closed this Sep 23, 2023
@QuLogic QuLogic deleted the llvm17 branch September 23, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants