Move `SECP256K1_INLINE` macro definition out from `include/secp256k1.h` #1231

pull hebasto wants to merge 2 commits into bitcoin-core:master from hebasto:230131-inline changing 15 files +29 −13
  1. hebasto commented at 12:10 PM on March 9, 2023: member

    From IRC:

    06:29 < hebasto> What are reasons to define the SECP256K1_INLINE macro in user's include/secp256k1.h header, while it is used internally only? 06:32 < hebasto> I mean, any other (or a new dedicated) header in src looks 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:

    • it is a step in direction to better organized headers (in context of #924, #1039)

    Cons:

    • code duplication for SECP256K1_GNUC_PREREQ macro
  2. real-or-random commented at 12:57 PM on March 9, 2023: contributor

    Oh, the CI failures revealed a bug. The examples shouldn't use SECP256K1_INLINE.

  3. Remove `SECP256K1_INLINE` usage from examples 77445898a5
  4. hebasto force-pushed on Mar 9, 2023
  5. hebasto commented at 1:06 PM on March 9, 2023: member

    Oh, the CI failures revealed a bug. The examples shouldn't use SECP256K1_INLINE.

    Thanks! Fixed.

  6. in src/util.h:21 in 5678ff7dd3 outdated 
      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__)
    real-or-random commented at 1:19 PM on March 9, 2023:

    formatting differs from the copy in the public header

    hebasto commented at 1:27 PM on March 9, 2023:

    Cannot see any difference. What did I miss?

    real-or-random commented at 2:10 PM on March 9, 2023:

    Oh, you're right... I was looking at the missing whitespace around &&, 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.

  7. real-or-random commented at 1:21 PM on March 9, 2023: contributor

    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.

  8. hebasto commented at 1:31 PM on March 9, 2023: member

    There's no strict reason why util.h shouldn't be able to include secp256k1.h...

    Wouldn't that be exposing of implementation details in a public interface?

  9. real-or-random commented at 2:09 PM on March 9, 2023: contributor

    There's no strict reason why util.h shouldn't be able to include secp256k1.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?

  10. hebasto commented at 3:25 PM on March 9, 2023: member

    There's no strict reason why util.h shouldn't be able to include secp256k1.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?

    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.~

  11. Move `SECP256K1_INLINE` macro definition out from `include/secp256k1.h` 8e142ca410
  12. hebasto force-pushed on Mar 9, 2023
  13. hebasto commented at 3:32 PM on March 9, 2023: member

    There's no strict reason why util.h shouldn't be able to include secp256k1.h, if it uses macro from the latter.

    Thanks! Done.

  14. real-or-random approved
  16. sipa commented at 3:52 PM on April 20, 2023: contributor

    utACK 8e142ca4102ade1b90dcb06d6c78405ef3220599

  17. real-or-random merged this on Apr 20, 2023
  18. real-or-random closed this on Apr 20, 2023
  19. hebasto deleted the branch on Apr 20, 2023
  20. sipa referenced this in commit b4eb644b6c on May 12, 2023
  21. hebasto referenced this in commit 49c52ea2b1 on May 13, 2023
  22. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  23. real-or-random referenced this in commit 0702ecb061 on Jun 21, 2023
  24. sipa referenced this in commit 901336eee7 on Jun 21, 2023
  25. vmta referenced this in commit 8f03457eed on Jul 1, 2023
  26. delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
  27. delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
  28. janus referenced this in commit c4348d88db on Sep 11, 2023
  29. div72 referenced this in commit af627d47c3 on Apr 12, 2025
  30. str4d referenced this in commit 5a6bf5f178 on Jun 4, 2025
  31. Fabcien referenced this in commit 8f5386e155 on Apr 17, 2026
  32. Fabcien referenced this in commit fd11bd5e98 on Apr 17, 2026
Contributors
hebastoreal-or-randomsipa
Linked (view graph)
#924 Refactor includes#1039 Make all source and header files self-contained#1338 Drop no longer needed `#include "../include/secp256k1.h"`
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC