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

Unable to wrap stat #320

Closed
melonedo opened this issue Aug 8, 2021 · 9 comments · Fixed by #321
Closed

Unable to wrap stat #320

melonedo opened this issue Aug 8, 2021 · 9 comments · Fixed by #321

Comments

@melonedo
Copy link
Contributor

melonedo commented Aug 8, 2021

This happened when I am wrapping GDAL.jl.
On linux, if I attemp to wrap this code:

#include <sys/stat.h>
typedef struct stat mystat;

because stat is also a function, Audit pass can not be passed. But it seems that neither CollectTopLevelNode nor CollectDependentSystemNode supports filtering, and Audit pass can not be disabled.
Do we need to add filtering earlier to solve this?

@Gnimuc
Copy link
Member

Gnimuc commented Aug 8, 2021

Is it feasible to use @add_def stat to skip? Anyway, we can not define a struct and a function with the same name in Julia.

@Gnimuc
Copy link
Member

Gnimuc commented Aug 8, 2021

related: #290

IMHO, this is a bug and should be fixed in the upstream.

@melonedo
Copy link
Contributor Author

melonedo commented Aug 8, 2021

Is it feasible to use @add_def stat to skip?

Currently resolve_dependency! and collect_dependent_system_nodes! for TypedefElaborated do not respect dag.ids_extra. They only check dag.tags. I modified them and it seems good.

Edit: oops, now struct tm is not wrapped.

@Gnimuc
Copy link
Member

Gnimuc commented Aug 9, 2021

Maybe dag.ids_extra needs to be checked firstly, so @add_def takes the highest priority.

@visr
Copy link
Contributor

visr commented Aug 10, 2021

Just to make sure, this is not because stat is being excluded in this rewriter?
https://github.com/JuliaGeo/GDAL.jl/blob/d44dc737ec16a60d1edb81bb71e0cb5f0666f165/gen/wrap.jl#L71

EDIT:
Nevermind, I see this errors before rewriting, getting

ERROR: LoadError: sanity check failed. [REASON]: use the same name _stat64 for struct CLCursor (CLStructDecl) _stat64 at C:\Users\visser_mn\.julia\artifacts\217a1339fc81b7388fc8af3fc8a05bca7b7ced19\x86_64-w64-mingw32\sys-root\include\_mingw_stat64.h:83:10 and function CLCursor (CLFunctionDecl) _stat64(const char *, struct _stat64 *) at C:\Users\visser_mn\.julia\artifacts\217a1339fc81b7388fc8af3fc8a05bca7b7ced19\x86_64-w64-mingw32\sys-root\include\sys/stat.h:97:23. The C code use the same name for structs and functions, which is not a good code-style, please fix it in the upstream.
Stacktrace:
 [1] error(s::String)
   @ Base .\error.jl:33
 [2] sanity_check(dag::ExprDAG, options::Dict{String, Any})
   @ Clang.Generators C:\Users\visser_mn\.julia\packages\Clang\Sf6js\src\generator\audit.jl:76
 [3] (::Audit)(dag::ExprDAG, options::Dict{String, Any})
   @ Clang.Generators C:\Users\visser_mn\.julia\packages\Clang\Sf6js\src\generator\passes.jl:815
 [4] build!(ctx::Context, stage::Clang.Generators.BuildStage)
   @ Clang.Generators C:\Users\visser_mn\.julia\packages\Clang\Sf6js\src\generator\context.jl:167
 [5] top-level scope
   @ C:\Users\visser_mn\.julia\dev\GDAL\gen\wrap.jl:208
 [6] include(fname::String)
   @ Base.MainInclude .\client.jl:444
 [7] top-level scope
   @ REPL[5]:1
in expression starting at C:\Users\visser_mn\.julia\dev\GDAL\gen\wrap.jl:208

Getting this error both on the #321 and dump-doc-patch branch mentioned in JuliaGeo/GDAL.jl#120

add https://github.com/melonedo/Clang.jl/#dump-doc-patch
add https://github.com/melonedo/Clang.jl/#patch2

@visr
Copy link
Contributor

visr commented Aug 10, 2021

IMHO, this is a bug and should be fixed in the upstream.

Just shooting here, but could this be related? OSGeo/gdal#4123

@melonedo
Copy link
Contributor Author

I think real C compilers probably should not error because of this. Here Clang.jl found stat to be a dependency and tries to pull it out from system headers. However, it can not distinguish stat (a function, not actually used) from struct stat and includes both, which causes sanity check to fail.
As for your case, I only tested on linux so probably more symbols (e.g. _stat64) need to be ignored and added back in prologue.jl.
I should have mentioned earlier that these checks are ignored because I disabled macros as whole, and stat and tm are special-cased.

@Gnimuc
Copy link
Member

Gnimuc commented Aug 10, 2021

One workaround is we always collect tag types and ignore functions.

@Gnimuc
Copy link
Member

Gnimuc commented Sep 14, 2021

fixed on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants