SECP256K1_INLINE macro definition out from include/secp256k1.h
#1231
From IRC:
06:29 < hebasto> What are reasons to define the
SECP256K1_INLINEmacro in user’sinclude/secp256k1.hheader, while it is used internally only? 06:32 < hebasto> I mean, any other (or a new dedicated) header insrclooks more appropriate, no? 06:35 < sipa> I think it may just predate any “utility” internal headers. 06:42 < sipa> I think it makes sense to move it to util.h
Pros:
Cons:
SECP256K1_GNUC_PREREQ macro
SECP256K1_INLINE.
Oh, the CI failures revealed a bug. The examples shouldn’t use
SECP256K1_INLINE.
Thanks! Fixed.
16@@ -17,6 +17,27 @@
17 #define DEBUG_CONFIG_MSG(x) "DEBUG_CONFIG: " x
18 #define DEBUG_CONFIG_DEF(x) DEBUG_CONFIG_MSG(#x "=" STR(x))
19
20+# if !defined(SECP256K1_GNUC_PREREQ)
21+# if defined(__GNUC__)&&defined(__GNUC_MINOR__)
&&, which should be added. For some reason, I believed it’s there in the public header, but I was probably looking at the wrong macro.
Concept ACK
That’s a breaking change technically, but in this case, even I tend not to treat it as one… :see_no_evil:
There’s no strict reason why util.h shouldn’t be able to include secp256k1.h, if it uses macro from the latter. If there’s a reason to consider that a bad idea (but I don’t think so), and we prefer code duplication, we should probably add a comment to each copy, indicating that there’s another copy that should be updated in lock-step.
There’s no strict reason why
util.hshouldn’t be able to includesecp256k1.h…
Wouldn’t that be exposing of implementation details in a public interface?
There’s no strict reason why
util.hshouldn’t be able to includesecp256k1.h…Wouldn’t that be exposing of implementation details in a public interface?
I meant adding #include "../include/secp256k1.h" to util.h, and not the other way around? Or how would that expose details?
There’s no strict reason why
util.hshouldn’t be able to includesecp256k1.h…Wouldn’t that be exposing of implementation details in a public interface?
I meant adding
#include "../include/secp256k1.h"toutil.h, and not the other way around? Or how would that expose details?
You’re right. I misunderstood you.
~Well, it would work but it requires to add include/ into include directories in build systems (-I$(top_srcdir)/include in Automake, ${PROJECT_SOURCE_DIR}/include in CMake) like we do for examples now.~
There’s no strict reason why
util.hshouldn’t be able to includesecp256k1.h, if it uses macro from the latter.
Thanks! Done.
