Minor improvements for better Rust compatibility #724
Conversation
nkaskov
changed the title
|
@nkaskov Just want to confirm that this change is needed for Rust compatibility. I see changes in some example, implying that the user API has changed (unexpected behavior for v1.1.5), which may lead to compilation errors for existing projects based on OpenFHE. Maybe it would be safer to add "is_allocated" w/o removing "good", so that the code would backwards compatible. In general, we only change API during major releases. Is this change required for the Rust bindings? |
Hi @yspolyakov! Thanks for your contribution. We decided to go with proposed solution and added |
nkaskov
force-pushed
the
main
branch
2 times, most recently
from
c2c90af
to
5694273
Compare
nkaskov
changed the title
| @@ -56,6 +56,7 @@ class CCParams<CryptoContextCKKSRNS> : public Params { | |||
| public: | |||
| CCParams() : Params(CKKSRNS_SCHEME) {} | |||
| explicit CCParams(const std::vector<std::string>& vals) : Params(vals) {} | |||
| explicit CCParams(const Params& params) : Params(params) {} | |||
There was a problem hiding this comment.
We do not need this constructor
| @@ -56,6 +56,7 @@ class CCParams<CryptoContextBGVRNS> : public Params { | |||
| public: | |||
| CCParams() : Params(BGVRNS_SCHEME) {} | |||
| explicit CCParams(const std::vector<std::string>& vals) : Params(vals) {} | |||
| explicit CCParams(const Params& params) : Params(params) {} | |||
There was a problem hiding this comment.
We do not need this constructor
| @@ -56,6 +56,7 @@ class CCParams<CryptoContextBFVRNS> : public Params { | |||
| public: | |||
| CCParams() : Params(BFVRNS_SCHEME) {} | |||
| explicit CCParams(const std::vector<std::string>& vals) : Params(vals) {} | |||
| explicit CCParams(const Params& params) : Params(params) {} | |||
There was a problem hiding this comment.
We do not need this constructor
| @@ -0,0 +1,136 @@ | |||
| //================================================================================== | |||
There was a problem hiding this comment.
I am aware about this situation, but didn't have time to fix it. it will be resolved differently, without creating a new file. just a few line code change
…ward compatibility, but made const.
…rom any other headers.
|
a new PR has been submitted instead of this one. see it here: #755 |
In this PR, we bring minor interface improvements in order to achieve better compatibility with the Rust binding we are building.
The changes brought by this PR:
explicitconstuctor to theCCParamsclasses. This allows for the creation of instances of these classes using only theParamsclass.