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

Update wrapper generator to Clang.jl master #119

Merged
merged 2 commits into from
Aug 29, 2021
Merged

Conversation

melonedo
Copy link
Contributor

@melonedo melonedo commented Aug 28, 2021

On next release, Clang.jl will release a new generator format, this updates the wrapper generator to the new format, and makes uses of JLL packages instead of manually copying headers. Compared using https://github.com/melonedo/WrapperComparator, the new wrapper will be identical to the old one.

cc @Gnimuc.

@codecov
Copy link

codecov bot commented Aug 28, 2021

Codecov Report

Merging #119 (d1bba18) into master (4d20d41) will increase coverage by 34.12%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #119       +/-   ##
===========================================
+ Coverage   55.66%   89.78%   +34.12%     
===========================================
  Files           3        2        -1     
  Lines         636      333      -303     
===========================================
- Hits          354      299       -55     
+ Misses        282       34      -248     
Impacted Files Coverage Δ
src/Clp.jl 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d20d41...d1bba18. Read the comment docs.

lib/libClp.jl Show resolved Hide resolved
lib/libClp.jl Show resolved Hide resolved
Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

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

LGTM

@odow odow merged commit 54ce473 into jump-dev:master Aug 29, 2021
@odow
Copy link
Member

odow commented Aug 29, 2021

Thanks!

As a side note: why'd you pick Clp? Just for test cases for Clang? Or are you interested in optimization? I ask because there are a lot of other solvers in jump-dev that have similar wrappers. But upstream is infrequently updated so there's no rush to update them as well.

@melonedo
Copy link
Contributor Author

I update them because we are preparing a new Clang.jl release as in JuliaInterop/Clang.jl#299. Also this is part of my OSPP project.
Clp is picked because I found its wrapper is not fully automatic.

Clp.jl/scripts/clang.jl

Lines 1 to 10 in 9a0b9d1

# TODO(odow):
#
# This script can be used to build the C interface to Clp. However, it requires
# you to manually do the following steps first:
#
# 1) Copy Clp_C_Interface.h from clp into this /scripts directory
# 2) Copy Coin_C_defines.h from coinutils into this /scripts directory
#
# It should be possible to build the wrapper using the jll's, but I couldn't
# figure out how to do that, and I didn't have enough time to spend on it.

@odow
Copy link
Member

odow commented Aug 29, 2021

Very cool. I'll comment in the other issue, but don't let the jump-dev repos block the release. We can update them as needed.

@odow odow mentioned this pull request Sep 29, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants