scipr-lab / libff

C++ library for Finite Fields and Elliptic Curves
Other
149 stars 82 forks source link

Changed static member function that was manipulating non-static members #93

Closed AntoineRondelet closed 3 years ago

AntoineRondelet commented 3 years ago

Description

The current state of develop does not compile in DEBUG mode because a static member function manipulates non-static members causing a compilation error.

Here is the log:

~/D/s/l/build ❯❯❯ make
[  6%] Built target zm
Scanning dependencies of target ff
[  8%] Building CXX object libff/CMakeFiles/ff.dir/algebra/curves/edwards/edwards_pairing.cpp.o
<path-to-libff>/scipr-lab/libff/libff/algebra/curves/edwards/edwards_pairing.cpp:251:20: fatal error: invalid use of member 'T' in static member function
            assert(T*Z == X*Y);
                   ^
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
     (static_cast <bool> (expr)                                         \
                          ^~~~
1 error generated.
libff/CMakeFiles/ff.dir/build.make:290: recipe for target 'libff/CMakeFiles/ff.dir/algebra/curves/edwards/edwards_pairing.cpp.o' failed
make[2]: *** [libff/CMakeFiles/ff.dir/algebra/curves/edwards/edwards_pairing.cpp.o] Error 1
CMakeFiles/Makefile2:755: recipe for target 'libff/CMakeFiles/ff.dir/all' failed
make[1]: *** [libff/CMakeFiles/ff.dir/all] Error 2
Makefile:160: recipe for target 'all' failed
make: *** [all] Error 2

Here are the problematic lines: https://github.com/scipr-lab/libff/blob/develop/libff/algebra/curves/edwards/edwards_pairing.cpp#L452-L471 (same for mnt4/mnt6) The error is triggered because of https://github.com/scipr-lab/libff/blob/develop/libff/algebra/curves/edwards/edwards_pairing.cpp#L505-L507 which is added to the code when we compile in debug mode only.

As it clearly seems that this invariant check function should not be a static one (it needs an instance to be called upon), I removed the static keyword. The compilation now passes in DEBUG mode


Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why.

solomonjoseph commented 3 years ago

On it Dev! Will have it done by EOD today.