Skip to content

Regenerate LibCURL.jl via Clang.jl #53

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

Merged
merged 13 commits into from
Sep 10, 2018
Merged

Conversation

nicoleepp
Copy link
Contributor

@nicoleepp nicoleepp commented Sep 7, 2018

The code used to regenerate is included in gen/generate.jl. Used Clang.jl on 0.7.

Closes #52

LibCURL and FTPClient tests pass.

Version of CURL/LibCURL used: 7.61.1

@omus
Copy link
Collaborator

omus commented Sep 7, 2018

The 0L literals are of type long and should be Clong(0) in Julia

@codecov-io
Copy link

codecov-io commented Sep 7, 2018

Codecov Report

Merging #53 into master will decrease coverage by 13.05%.
The diff coverage is 6.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #53       +/-   ##
==========================================
- Coverage      20%   6.94%   -13.06%     
==========================================
  Files           2       2               
  Lines          10      72       +62     
==========================================
+ Hits            2       5        +3     
- Misses          8      67       +59
Impacted Files Coverage Δ
src/LibCURL.jl 11.11% <ø> (ø) ⬆️
src/lC_curl_h.jl 6.34% <6.34%> (ø)

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 28f76b5...0b61859. Read the comment docs.

Copy link
Contributor Author

@nicoleepp nicoleepp left a comment

Choose a reason for hiding this comment

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

Not sure why the ANONYMOUS constants were generated or if they are needed (in lc_common_h.jl).
Edit: They have now been deleted since they came from badly parsed enums.

@nicoleepp
Copy link
Contributor Author

There is one constant I'm not sure what to do with. It gets generated as

const CURL_ZERO_TERMINATED               = ((size_t) -1)

but this errors in julia:

julia> Csize_t(-1)
ERROR: InexactError: check_top_bit(Int64, -1)
Stacktrace:
 [1] throw_inexacterror(::Symbol, ::Any, ::Int64) at ./boot.jl:567
 [2] check_top_bit at ./boot.jl:581 [inlined]
 [3] toUInt64 at ./boot.jl:692 [inlined]
 [4] UInt64(::Int64) at ./boot.jl:722
 [5] top-level scope at none:0

I have left the constant defined for now as:

const CURL_ZERO_TERMINATED               = -1

@nicoleepp
Copy link
Contributor Author

nicoleepp commented Sep 10, 2018

My other concern is that some things that used to be Union{} are now just Cvoid, and constants that were Ints are now specified as UInt32. Not sure if these changes are what we want or if it's just how things are generated.

@omus
Copy link
Collaborator

omus commented Sep 10, 2018

The C code probably wants to do:

julia> reinterpret(Csize_t, -1)
0xffffffffffffffff

@nicoleepp nicoleepp changed the title WIP: regenerate LibCURL.jl with newer version of libcurl Regenerate LibCURL.jl Sep 10, 2018
@nicoleepp
Copy link
Contributor Author

I removed the c and ctypedef macros because we would have to manually go through the generated code and change the format to use them, but, if it makes for easier reviewing, I can add them back and re-format the definitions.

Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

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

Just some minor stylistic pedantry 😄

Copy link
Collaborator

@omus omus left a comment

Choose a reason for hiding this comment

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

Looking good

@omus
Copy link
Collaborator

omus commented Sep 10, 2018

@nicoleepp I assume you're good with squashing the commits?

@omus
Copy link
Collaborator

omus commented Sep 10, 2018

Travis also passed but seems to have not linked back to this PR

@nicoleepp
Copy link
Contributor Author

I'm good with squashing

@omus omus changed the title Regenerate LibCURL.jl Regenerate LibCURL.jl via Clang.jl Sep 10, 2018
@omus omus merged commit 97f22f0 into JuliaWeb:master Sep 10, 2018
@ararslan
Copy link
Member

Nice work here!

@nicoleepp nicoleepp deleted the ne/generate-deps branch September 10, 2018 19:46
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.

4 participants