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

Set version 0.3.3 && update third-party packages versions && downgrade msys2 && compatibility with now openfhe-development #52

Merged
merged 12 commits into from
Jun 27, 2024

Conversation

ArseniyKholod
Copy link
Collaborator

@ArseniyKholod ArseniyKholod commented Jun 26, 2024

I set new version and also update versions of third-party libraries in ci.yml, I forgot to do it last time

@ArseniyKholod ArseniyKholod changed the title Set version 0.3.3 Set version 0.3.3 && update third-party packages versions Jun 26, 2024
@ArseniyKholod ArseniyKholod changed the title Set version 0.3.3 && update third-party packages versions Set version 0.3.3 && update third-party packages versions && downgrade msys2 Jun 26, 2024
@ArseniyKholod
Copy link
Collaborator Author

As you can observe, now our CI on MacOs fails. In the update of openfhe-development in lbcrypto::Params class all setters were marked as virtual, but destructor was leaved as it was, so non-virtual. It causes a warning and error while compiling with Clang. I opened an issue openfheorg/openfhe-development#825 to understand, whether this non-virtual destructor is desired. This also leads to the same issue with derived lbcrypto::CCParams<...> classes.

So what we can do is:

  1. Disable -Werrors, so do not treat warnings as errors if this non-virtual destructor is not desired and openfhe-development will got updated soon with fixing this bug.
  2. Find a way around this issue by asking in CxxWrap.jl community.
  3. Do not migrate to v1.2.0 before this issue will be solved, if it will be

@sloede
Copy link
Member

sloede commented Jun 26, 2024

Thanks for the detailed analysis. What I don't understand though is why you think it's an issue with CxxWrap.jl - to me it seems to be the combination of a newer clang version with OpenFHE.

What about trying to silence the warning for now on macOS by passing -Wno-delete-non-abstract-non-virtual-dtor as a compile argument?

@ArseniyKholod ArseniyKholod changed the title Set version 0.3.3 && update third-party packages versions && downgrade msys2 Set version 0.3.3 && update third-party packages versions && downgrade msys2 && compatibility with now openfhe-development Jun 26, 2024
@ArseniyKholod
Copy link
Collaborator Author

ArseniyKholod commented Jun 26, 2024

Thanks for the detailed analysis. What I don't understand though is why you think it's an issue with CxxWrap.jl - to me it seems to be the combination of a newer clang version with OpenFHE.

What about trying to silence the warning for now on macOS by passing -Wno-delete-non-abstract-non-virtual-dtor as a compile argument?

No, it is of course not an issue with Cxxwrap.jl, but theoretically non-virtual destructor in virtual class is not an error, if no one tries to deallocate the derived class object by the pointer to the virtual class, so there is should be way to wrap normally...

I deleted -Werror and it works, but yes, it is better to disable one particular warning, thanks!

I disabled the warning and it is compiled successfully! I don't think it is a problem of new version of clang, this warning was added not in clang 15, but before, and we already used in previous commits this clang versions, it was ok, I also tested it in #53. But it is always so difficult with compilers...)

Should merge this PR as it is and update Yggdrasil?

CMakeLists.txt Outdated Show resolved Hide resolved
Co-authored-by: Michael Schlottke-Lakemper <[email protected]>
@ArseniyKholod ArseniyKholod merged commit bf374dd into hpsc-lab:main Jun 27, 2024
7 checks passed
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.

2 participants