clang compatibility fixes #71
Conversation
| @@ -60,8 +60,8 @@ struct knowledge_commitment { | |||
| template<typename T1, typename T2, mp_size_t m> | |||
| knowledge_commitment<T1,T2> operator*(const libff::bigint<m> &lhs, const knowledge_commitment<T1,T2> &rhs); | |||
|
|
|||
| template<typename T1, typename T2, mp_size_t m, const libff::bigint<m> &modulus_p> | |||
| knowledge_commitment<T1,T2> operator*(const libff::Fp_model<m, modulus_p> &lhs, const knowledge_commitment<T1,T2> &rhs); | |||
| template<typename T1, typename T2, typename T> | |||
There was a problem hiding this comment.
This less-specific templatization loses information, and could cause future problems overloading operator* for other types. The change assumes that anything you'd multiply by a knowledge_commitment would have an as_bigint() method.
Does the original exceed C++11, or is clang (and which version?) showing incomplete C+1+11 support here?
There was a problem hiding this comment.
Agree, this causes me to feel uneasiness as well, maybe we can figure out a compromise.
As far as it seems to me, it's a different interpretation regarding non-type template parameter deduction - gcc decides to infer "bigint" from "const bigint &" while clang is being more strict and decides not to.
This is a contained example that shows the same behavior:
http://coliru.stacked-crooked.com/a/583904c911c2e84b
| @@ -69,8 +69,8 @@ knowledge_commitment<T1,T2> operator*(const libff::bigint<m> &lhs, const knowled | |||
| lhs * rhs.h); | |||
| } | |||
|
|
|||
| template<typename T1, typename T2, mp_size_t m, const libff::bigint<m> &modulus_p> | |||
| knowledge_commitment<T1,T2> operator*(const libff::Fp_model<m, modulus_p> &lhs, const knowledge_commitment<T1,T2> &rhs) | |||
| template<typename T1, typename T2, typename T> | |||
|
Any progress on this? |
This PR together with the one in libff has some solution for clang compatibility.
Specifically, it handles issues mentioned in #54 and #67, about the following: