-
Notifications
You must be signed in to change notification settings - Fork 29
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
clang>=16 is not supported #139
Comments
I actually doubt that this is a toolchain issue. Could you first verify that you can compile the released generated sources with your toolchain to confirm/reject my doubt? |
I cannot find any repo with the pywrap generated C++ files in the OCP subdir, can you kindly point to where I can find it? |
By using the released stubs OCP_src_stubs_ubuntu-20.04.zip you suggested. the building starts but I get another error:
Furthermore by comparing the OCP sources between the released archive and the pywrap generated ones, they look quite different, for instance in the NCollection_tmpl.hxx file all the iterator related methods in register_template_NCollection_Sequence() are not present (in OCP_src_stubs_ubuntu-20.04.zip) |
It looks like that shipped bindings OCP_src_stubs_ubuntu-20.04.zip were generated from an unpublished opencascade version (same error reported in #138). The function IsTangentFaces() for opencascade-7.7.2 has only four arguments |
Interesting, in the version that was used (occt 7.7.2 available in in the conda-forge channel) I see this: //! creation of spatial fillets on a solid.
class ChFi3d
{
public:
DEFINE_STANDARD_ALLOC
//! Defines the type of concavity in the edge of connection of two faces
Standard_EXPORT static ChFiDS_TypeOfConcavity DefineConnectType (const TopoDS_Edge& E,
const TopoDS_Face& F1,
const TopoDS_Face& F2,
const Standard_Real SinTol,
const Standard_Boolean CorrectPoint);
//! Returns true if theEdge between theFace1 and theFace2 is tangent
Standard_EXPORT static Standard_Boolean IsTangentFaces (const TopoDS_Edge& theEdge,
const TopoDS_Face& theFace1,
const TopoDS_Face& theFace2,
const GeomAbs_Shape Order = GeomAbs_G1);
Standard_EXPORT static Standard_Boolean IsTangentFaces (const TopoDS_Edge& theEdge,
const TopoDS_Face& theFace1,
const TopoDS_Face& theFace2,
Standard_Real G1Tol,
const GeomAbs_Shape Order = GeomAbs_G1); So two overloads. I do not understand what happened here. |
The change has been introduced here, maybe the author @Michsior14 can explain while the occt code in conda-forge is diverging from upstream. If that is really important because it solves an important issue, than I would strongly suggest to integrate the patch upstream! |
@adam-urbanczyk the building error of this issue is related to an incomplete parsing of the API by pywrap. Here I am guessing because I am far from being a C++ expert. If the class methods are using local types (e.g. the For instance:
should become
in the output file Another similar error, this time
here the argument to |
Yes, that sounds more like it. If this one overload is blocking, I can update |
You would need to ask bloblfish or occt-feedstock maintainers. I've only updated the patch to v7.8.0 (it's been introduced some versions ago already). |
already opened an issue |
@efferre79 any thoughts/updates?
|
@adam-urbanczyk sorry for the latency but up to now I was trying to patch pywrap to improve the generated code from the current ocp.toml. Here you can find the log I get during the source configuration done by cmake. Let me know if that is what you asked for. |
@adam-urbanczyk I have created a starting point to modify pywrap and fixing at least partially this issue, if you have time have a look at my branch innertypes. Right now I am stuck because I am facing something weird probably depending on a libclang bug, the error I get is the following:
I have written a stupid python script to demonstrate the problem, the parsing of the args of AppDef_HArray1OfMultiPointConstraint class constructors is not correct and leads to the error above. I am using clang-17 |
In the log dump that you shared you have this:
You'll need to specify correct/additional (I'm not sure what they are) include dirs for your env. |
Maybe #140 solves the issue, let's see |
Almost, on my system I had to further patch the
But I would avoid forcing CXX_INCLUDES value with CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES (there is a reason they are not set, see https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_IMPLICIT_INCLUDE_DIRECTORIES.html). When gcc is used as compiler, it already knows them so it is not needed. I will try later with clang as CXX. So I think it is better to provide them to pywrap execution only, e.g. defining a new PYWRAP_CXX_INCLUDES variable. Furthermore your #140 (patched as told above) is solving also my issue with AppDef_HArray1OfMultiPointConstraint class constructors described here. I still get another error in the methods definition for non templates but probably that can be solved by further improving the patch of my pywrap branch. Working on it right now. |
AFAIK |
Yes, you are right. The name misled me, sorry for the noise. |
hello @adam-urbanczyk, I have improved the pywrap code to almost successfully compile OCP. You can find the changes in my innertypes branch. I haven't created a pull request yet because only one missing error must be solved but I don't know what to do from the C++ point of view. The error I get is the following:
where of course the So currently I added Moreover this is the difference in the shipped includes vs upstream files, wondering if something else should be added to ocp.toml:
|
Too many changes at once, I'm afraid. What is the problem you are facing, can you generate code without warnings other than seen in the CI. If so, what is the delta of the generated code wrt to the released one [other than related to the patches of blobfish]? |
here is the diff between the generated bindings (which don't compile) and the contents of OCP_src_stubs_ubuntu-20.04.zip |
How about warnings? |
Here is the output from cmake
|
@adam-urbanczyk a quick update of what I discovered. First of all I have finally managed to build OCP with clang 14 and 15. But this bug is still valid when using clang 16 or greater. See also below. I am using a Gentoo-based system, they partially backported the clang 16 functionality of system config files in clang15. So building OCP with clang 15 didn't work out of the box on my system because the system config files have some Then the bad news. Unfortunately, starting from clang version 16 the behaviour is changed (probably the reason is explained here) so it is not possible to get the same kind of information required for types qualification of function arguments/return values. |
Thanks, so for the time being the solution is to use the configuration specified in this repo (i.e. clang 15). I do not understand the story with |
The commit using
you can see if clang supports system configuration files. This is enabled upstream from clang-16, on Gentoo since clang-15. On Gentoo, they addressed another issue and since then one of the config files contains a clang If you decide to remove the previous commit, I would strongly suggest to make pywrap more robust. For instance you can filter out the invalid HeaderInfo() objects or printing a warning or in alternative, you could foresee an user defined clang command line option to be added when parsing the translation units. |
Alright, I'll update the README to say something about the deps (i.e. essentially take the versions from the devenv file or be on your own). Would you happen to know why includes added via Last, but not least, shall I get rid of the problematic overload coming from the blobfish patch? AFAICT this way everyone will be able to use the generated sources directly? Does that fit your use case? |
I have no idea.
I am using directly opencascade headers from my distribution and I don't use conda at all. In my opinion having to ship the generated sources is quite limiting, it should be a backup option and considered as last option. To be honest I don't see the reason of keeping blobfish patch, even the maintainers don't know what is its purpose. The most famous software using opencascade seems to be freecad and its build system can be used directly with upstream opencascade. |
I am using the CMakeLists.txt file from the repository, I have successfully created the bindings using clang 17, pywrap from master branch and pybind 2.11.1 but later I get many errors when starting to build. I am here reporting just the first lines from the build failure:
I am using GCC 12 and opencascade 7.7.2. I have also tried using gcc-8 or clang to build the CXX code without success.
@adam-urbanczyk which are the versions of build tools you suggest to use?
The text was updated successfully, but these errors were encountered: