cmake: Add SECP256K1_LATE_CFLAGS configure option

[hebasto]

This PR enables users to override compiler flags that have been set by the CMake-based build system, such as warning flags.

The Autotools-based build system has the same feature out-of-the-box.

See more details here.

Here are some examples of the new option usage:

cmake -S . -B build -DSECP256K1_LATE_CFLAGS="-Wno-extra -Wlong-long"

cmake -S . -B build -DSECP256K1_BUILD_EXAMPLES=ON -DSECP256K1_LATE_CFLAGS=-O1
cmake --build build
...
In function ‘secp256k1_ecmult_strauss_wnaf’,
    inlined from ‘secp256k1_ecmult’ at /home/hebasto/git/secp256k1/src/ecmult_impl.h:353:5:
/home/hebasto/git/secp256k1/src/ecmult_impl.h:291:5: warning: ‘aux’ may be used uninitialized [-Wmaybe-uninitialized]
  291 |     secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/hebasto/git/secp256k1/src/secp256k1.c:29:
/home/hebasto/git/secp256k1/src/ecmult_impl.h: In function ‘secp256k1_ecmult’:
/home/hebasto/git/secp256k1/src/group_impl.h:174:13: note: by argument 3 of type ‘const secp256k1_fe *’ to ‘secp256k1_ge_table_set_globalz’ declared here
  174 | static void secp256k1_ge_table_set_globalz(size_t len, secp256k1_ge *a, const secp256k1_fe *zr) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/hebasto/git/secp256k1/src/secp256k1.c:30:
/home/hebasto/git/secp256k1/src/ecmult_impl.h:345:18: note: ‘aux’ declared here
  345 |     secp256k1_fe aux[ECMULT_TABLE_SIZE(WINDOW_A)];
      |                  ^~~
...

Please note that in the last case providing env CFLAGS=-O1 or -DCMAKE_C_FLAGS=-O1 won't work.

[real-or-random]

Concept ACK on giving the user the last word

This seems to be a major pain point of CMake for packaging:
https://wiki.archlinux.org/title/CMake_package_guidelines#CMake_can_automatically_override_the_default_compiler_optimization_flag
I really don't understand why they don't have native ways of prepending flags (instead of appending), or why there's no commonly accepted practice here.

I'm not convinced that -DSECP256K1_C_FLAGS is the best option. It solves the problem, but it's hard to discover/non-standard. After having read that piece in the Arch Wiki, maybe we should simply make sure that -DCMAKE_BUILD_TYPE=None does not set potentially undesirable flags that the user may want to override? (Though it's not clear if we should drop all flags then, e.g., what about warning flags?)

Or perhaps we should make sure that -DCMAKE_BUILD_TYPE=None does not set -O or -g but just warnings (I assume this is the behavior already now?), and still add a SECP256K1_C_FLAGS variable as a last resort (also useful for development, debugging etc.)

@theuni What's your opinion on this?

[hebasto]

Or perhaps we should make sure that -DCMAKE_BUILD_TYPE=None does not set -O or -g but just warnings (I assume this is the behavior already now?)

Yes. That is the current behavior.

[real-or-random]

Or perhaps we should make sure that -DCMAKE_BUILD_TYPE=None does not set -O or -g but just warnings (I assume this is the behavior already now?), and still add a SECP256K1_C_FLAGS variable as a last resort (also useful for development, debugging etc.)

I think we should do this, but maybe rename it to SECP256K1_LATE_CFLAGS to emphasize that it's a special thing. (And I think CFLAGS is more common than C_FLAGS.)

[hebasto]

Or perhaps we should make sure that -DCMAKE_BUILD_TYPE=None does not set -O or -g but just warnings (I assume this is the behavior already now?), and still add a SECP256K1_C_FLAGS variable as a last resort (also useful for development, debugging etc.)

I think we should do this, but maybe rename it to SECP256K1_LATE_CFLAGS to emphasize that it's a special thing. (And I think CFLAGS is more common than C_FLAGS.)

Done.

[real-or-random]

Concept ACK

[real-or-random]

Couldn't we use add_compile_options instead? https://cmake.org/cmake/help/latest/command/add_compile_options.html

[hebasto]

It

adds options to the COMPILE_OPTIONS directory property.

which means the options won't be visible in the src subdirectory if added after add_subdirectory(src). Beeing added before add_subdirectory(src) they will be followed by the COMPILE_OPTIONS target properties added by target_compile_options().

[real-or-random]

Suggested change

	set(SECP256K1_LATE_CFLAGS "" CACHE STRING "Compiler flags that can override flags set by the build system.")
	set(SECP256K1_LATE_CFLAGS "" CACHE STRING "Compiler flags that are added to the command line after all other flags added by the build system.")

I think something like this is more precise. What the semantics of the flags are (i.e., whether earlier options can actually be overridden) depends on the compiler.

[hebasto]

Updated.

[theuni]

Concept ACK.

[real-or-random]

ACK 42f8c51

@hebasto Is there a specific reason to keep every function in a separate file? We could also just have a single file for all the util functions. I don't have a strong opinion, it just looks a bit uncommon to me to have separate files. But there's nothing wrong with it, I guess.

[hebasto]

@hebasto Is there a specific reason to keep every function in a separate file? ... I don't have a strong opinion, it just looks a bit uncommon to me to have separate files. But there's nothing wrong with it, I guess.

It is an accepted practice, for example, in https://github.com/llvm/llvm-project/.

We could also just have a single file for all the util functions.

How should that single file be named in such a case?

[real-or-random]

@hebasto Is there a specific reason to keep every function in a separate file? ... I don't have a strong opinion, it just looks a bit uncommon to me to have separate files. But there's nothing wrong with it, I guess.

It is an accepted practice, for example, in llvm/llvm-project.

Ok, I see, then let's not bother with it. :)