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

Upgrade wrapper generator script and regenerate bindings #186

Merged
merged 13 commits into from
Aug 10, 2021

Conversation

Gnimuc
Copy link
Contributor

@Gnimuc Gnimuc commented Mar 3, 2021

I'm testing JuliaInterop/Clang.jl#278. Before making a new release for Clang.jl, I'd like to make sure that the code generated by the new generator is not broken. Any feedback about the new generator would be highly appreciated.

@rschwarz
Copy link
Collaborator

rschwarz commented Mar 3, 2021

Note that the generation process was not totally automatic in our case, see #77 .

We also pinned CEnum to an older version (for no good reason except that the we didn't want to change the generated code) and later vendored the module.

@Gnimuc
Copy link
Contributor Author

Gnimuc commented Mar 3, 2021

With this PR, the generation process should be totally automatic.

Missing definitions are all macro-related and one can manually add them to prologue.jl.

Variadic functions can be supported by using Julia 1.5's @ccall macro. In the future, Clang.jl should handle this, but for now, they're handled in epilogue.jl.

@Gnimuc
Copy link
Contributor Author

Gnimuc commented Mar 3, 2021

We also pinned CEnum to an older version (for no good reason except that the we didn't want to change the generated code) and later vendored the module.

As SCIP doesn't use duplicated enum members, so I just completely remove the use of CEnum.jl. Clang.jl's new generator now has an option for this.

@rschwarz
Copy link
Collaborator

rschwarz commented Mar 3, 2021

OK, so LibSCIP.jl is now the only file generated automatically by the script?
And the manual adjustments are all covered in gen/prologue.jl, gen/epilogue.jl or src/wrapper.jl?

That's a great improvement and will help a lot to keep up with API changes in SCIP in the future!

@matbesancon
Copy link
Member

this is fantastic yes

gen/generator.toml Outdated Show resolved Hide resolved
gen/generator.toml Outdated Show resolved Hide resolved
@Gnimuc Gnimuc changed the title DO NOT MERGE: Upgrade wrapper generator script and regenerate bindings Upgrade wrapper generator script and regenerate bindings Apr 14, 2021
@Gnimuc
Copy link
Contributor Author

Gnimuc commented Apr 14, 2021

@matbesancon @rschwarz This is ready for another round of review. As for the test failures, I guess I did something wrong when solving the conflict. Feel free to edit this PR if needed. I'm going to merge the Clang.jl#278 this weekend and after that, you can run the generator on your own machine with Clang.jl#master.

@Gnimuc
Copy link
Contributor Author

Gnimuc commented May 12, 2021

I won't keep updating this PR but will keep monitoring the compatibility of the generated code here.

@matbesancon
Copy link
Member

thanks for the work on this! So the current PR is based on what is currently on Clang.jl master right?

@Gnimuc
Copy link
Contributor Author

Gnimuc commented May 13, 2021

Yes. It's recommended to use Clang.jl#master from now on.

@matbesancon
Copy link
Member

there seems to be some conversion error: unsafe_convert(::Type{Ptr{Ptr{Int8}}}, ::Base.RefValue{Cstring})

@Gnimuc
Copy link
Contributor Author

Gnimuc commented May 13, 2021

This has been fixed in eba7eff, but I did something wrong when solving the conflict, so it's not in the latest commit. I believe you could fix this by cherry-pick that commit or just simply fix it with a new one.

@matbesancon
Copy link
Member

@Gnimuc do you know when a release will be tagged on Clang.jl? Depending on master is a bit dangerous as next commits can break things

@Gnimuc
Copy link
Contributor Author

Gnimuc commented May 19, 2021

This package is being monitored in https://github.com/Gnimuc/GeneratorScripts, it's safe to just use the master branch.

@Gnimuc
Copy link
Contributor Author

Gnimuc commented May 19, 2021

Actually, this PR bundles the Modifest.toml, so it will always use the same version of Clang.jl unless someone does a pkg> up in the project mode.

@matbesancon matbesancon closed this Aug 9, 2021
@matbesancon matbesancon reopened this Aug 9, 2021
@matbesancon
Copy link
Member

@Gnimuc should we bump the gen/Project.toml to the latest 0.11 you released?

@Gnimuc
Copy link
Contributor Author

Gnimuc commented Aug 10, 2021

@Gnimuc should we bump the gen/Project.toml to the latest 0.11 you released?

0.11? you mean 0.14? No, 0.14 has not been officially released yet. The latest master has some new features like variadic function support, docstrings dumping, etc. If you'd like to update to the latest master, run pkg> up in the project mode.

@matbesancon matbesancon merged commit 7aa79aa into scipopt:master Aug 10, 2021
@matbesancon
Copy link
Member

ok thank you, we do not need this for now so merging

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.

None yet

3 participants