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

Some header files entirely skipped #240

Closed
IanButterworth opened this issue Oct 27, 2019 · 10 comments · Fixed by #278
Closed

Some header files entirely skipped #240

IanButterworth opened this issue Oct 27, 2019 · 10 comments · Fixed by #278

Comments

@IanButterworth
Copy link
Member

My libjpeg wrapper completes successfully, but on loading the output hits errors because of missing consts, the first of which is defined in jmorecfg.h and I noticed the following warnings. Why are those provided headers being skipped?

┌ Warning: not wrapping CLCursor (CLLastPreprocessing) jconfig.h
└ @ Clang ~/.julia/packages/Clang/CiPzM/src/wrap_c.jl:443
┌ Warning: not wrapping CLCursor (CLLastPreprocessing) jmorecfg.h
└ @ Clang ~/.julia/packages/Clang/CiPzM/src/wrap_c.jl:443

Wrapper code: https://github.com/ianshmean/ImageIO.jl/blob/master/gen/libjpeg/wrap_libjpeg.jl

@Gnimuc
Copy link
Member

Gnimuc commented Oct 28, 2019

Could you share the header file so I can debug this?

@IanButterworth
Copy link
Member Author

IanButterworth commented Oct 28, 2019

@Gnimuc
Copy link
Member

Gnimuc commented Oct 28, 2019

It looks like you forgot to add macro definitions to Clang's cmd arguments. e.g.
clang_args = ["-I", joinpath(LIB_INCLUDE, ".."), "-DJPEG_INTERNAL_OPTIONS"]

@IanButterworth
Copy link
Member Author

Ah, that helps bring a few more defs into _common but I'm still hitting a missing JSAMPLE type from jmorecfg.h at load time.

That's with:

clang_args = ["-I", joinpath(LIB_INCLUDE, ".."), "-DJPEG_INTERNALS", "-DJPEG_INTERNAL_OPTIONS"],

@Gnimuc
Copy link
Member

Gnimuc commented Oct 28, 2019

@IanButterworth
Copy link
Member Author

But that's defined in jconfig.h. Are those settings not read? or perhaps does the ordering of the header filepaths array matter?

@IanButterworth
Copy link
Member Author

Explicitly setting "-DBITS_IN_JSAMPLE=8" did work, btw.

I'm now hitting an odd bug.. these two structs reference each other.. which breaks loading in the wrapped version:

https://github.com/ianshmean/ImageIO.jl/blob/e7bbf7de0b6b12e58243f0ce54f0167244d84038/gen/libjpeg/ref_headers/jpeglib.h#L201-L210

header:

typedef struct jpeg_marker_struct *jpeg_saved_marker_ptr;

struct jpeg_marker_struct {
  jpeg_saved_marker_ptr next;   /* next in list, or NULL */
  UINT8 marker;                 /* marker code: JPEG_COM, or JPEG_APP0+n */
  unsigned int original_length; /* # bytes of data in the file */
  unsigned int data_length;     /* # bytes of data saved at data[] */
  JOCTET *data;                 /* the data contained in the marker */
  /* the marker length word is not counted in data_length or original_length */
};

Wrapped version:

const jpeg_saved_marker_ptr = Ptr{jpeg_marker_struct}

struct jpeg_marker_struct
    next::jpeg_saved_marker_ptr
    marker::UINT8
    original_length::UInt32
    data_length::UInt32
    data::Ptr{JOCTET}
end

??

@Gnimuc
Copy link
Member

Gnimuc commented Oct 28, 2019

Julia still cannot handle forward declaration yet(keno has a PR#32658 for this). the workaround is to use Ptr{Cvoid} instead of jpeg_saved_marker_ptr as the field type:

struct jpeg_marker_struct
    next::Ptr{Cvoid}
    marker::UINT8
    original_length::UInt32
    data_length::UInt32
    data::Ptr{JOCTET}
end

BTW, Julia do support some kinda "forward declaration" in the following way:

struct jpeg_marker_struct{_jpeg_saved_marker_ptr}
    next::_jpeg_saved_marker_ptr
    marker::UINT8
    original_length::UInt32
    data_length::UInt32
    data::Ptr{JOCTET}
end

Note that this only works if jpeg_marker_struct is not wrapped as a field type in other structs.

@IanButterworth
Copy link
Member Author

I see ok. I've switched in the Ptr{Cvoid} approach.

How do you recommend adjusting wrapped files? I'd like to make the wrap process repeatable, but the idea of doing a multiline sed doesn't seem ideal.

@Gnimuc
Copy link
Member

Gnimuc commented Oct 28, 2019

Since those forward decls are very easy to spot and there are only a few API changes between minor releases, I just use git and manually select and commit new changes when upgrading the lib to a newer minor version. To make the whole process fully automatic is cool, but maybe also a little bit overkill, I guess. :P

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 a pull request may close this issue.

2 participants