secp256k1 icon indicating copy to clipboard operation
secp256k1 copied to clipboard

No declaration for `secp256k1_xonly_pubkey_load`.

Open roconnor-blockstream opened this issue 4 years ago • 8 comments

secp256k1_xonly_pubkey_load is used in modules/schnorrsig/main_impl.h but there is declaration for it.

roconnor-blockstream avatar Jan 20 '21 21:01 roconnor-blockstream

Similarly secp256k1_pubkey_load is used in modules/extrakeys/main_impl.h but there is no declaration for it.

roconnor-blockstream avatar Jan 22 '21 18:01 roconnor-blockstream

Where should they be declared though? it seems like the only place it makes since is on top of the same main_impl.h file?

Should static functions even be declared before implementation?

elichai avatar Sep 15 '21 09:09 elichai

Please correct me if I'm wrong, but my understanding is that functions defined in one file but used in other files ought to have declarations.

roconnor-blockstream avatar Sep 15 '21 11:09 roconnor-blockstream

@roconnor-blockstream oh right, extrakeys and schnorrsig are separate files that depend on each other.

I guess this only works because of the order in which they are included in secp256k1.c

elichai avatar Sep 15 '21 12:09 elichai

Declarations are only necessary when calling a function defined in another compilation unit. Libsecp256k1 is compiled as a single compilation unit (even though some optional parts are called "modules"); everything is directly or indirectly #include'd from secp256k1.c. So only API functions (which can be called from outside the library) strictly speaking need declarations.

Declarations are also necessary for functions that are called before being defined, which happens e.g. when making mutually recursive calls. I don't think we have any instances of that in the codebase.

The compiler will warn if a function is called without definition, so I'm pretty sure everything is ok.

We do have declarations in e.g. field.h and group.h, but those just serve as compiler-checked developer documentation for the implementations and "interface" to that part of the codebase. They're redundant technically speaking.

sipa avatar Sep 15 '21 12:09 sipa

Right. My understanding is that secp256k1_xonly_pubkey_load and friends ought to have (but isn't required to have) "compiler-checked developer documentation for the implementations and 'interface' to that part of the codebase", and that is what this issue is about. If I'm wrong, then we should close this issue.

roconnor-blockstream avatar Sep 15 '21 13:09 roconnor-blockstream

Hm, that's a valid point. In general, we have only _impl.h files in the modules but no files for the internal API exposed by the modules, and schnorrsig depends on extrakeys for example.

real-or-random avatar Sep 15 '21 14:09 real-or-random

Right. My understanding is that secp256k1_xonly_pubkey_load and friends ought to have (but isn't required to have) "compiler-checked developer documentation for the implementations and 'interface' to that part of the codebase", and that is what this issue is about.

Oh, yes, I agree with that.

sipa avatar Sep 15 '21 14:09 sipa