Apply custom compilation flags only to 1P code

[tlively]

Our CMakeLists.txt previously added compile and link flags by updating
the CMAKE_CXX_FLAGS and similar global properties. This caused problems
when experimenting with adding new dependencies to Binaryen because our
custom compile flags, which largely enable stricter errors, were
incompatible with the third party code.

Simplify and improve our CMake configuration by applying extra
compilation flags only to code under our control. Do so by coalescing
all of the libbinaryen sources into a single target, then use CMake's
target_compile_options and target_link_options commands to set the flags
only for the libbinaryen and tool targets.

Also update the minimum required CMake version to avoid a policy error.

[tlively]

Woooo I finally got CI passing. @kripken, PTAL at the changes since your last review.

[kripken]

Is this enough for say Emscripten's requirements (both versions and current builders)?

[tlively]

I believe so. I took a look at Emscripten's CI config before making this change and as far as I could tell, it was using focal instead of bionic.

[kripken]

Suggested change

	option(BYN_ENABLE_LTO "Build with LTO" Off)
	option(BYN_ENABLE_LTO "Build with LTO" OFF)

[kripken]

can't a setting work as well? (I've never seen cmake properties used before, and at a glance at the usage and docs, it's not obvious to me what is added here, since this is global)

[tlively]

I'm not sure what you mean by a "setting." AFAIK cmake has no such concept. I can't use a normal variable because they can only be set in current or immediate parent scopes, so there's no way for binaryen_add_executable calls from subdirectories to modify a shared list of targets. The only other kind of "variable" cmake has is properties. This particular property is not stored in the cache because there's no reason for the user to have visibility into it.

[kripken]

By "setting" I mean e.g. if(NOT EMSCRIPTEN) that we use in several CMakeLists.txt files. Those can be set from the commandline but also using set() IIANM.

But, taking a step back here, doesn't CMake apply settings to child directories? What if we moved the flags stuff from the toplevel CMakeLists.txt to src/, which contains only our code?

[tlively]

Moving the flags management one scope down wouldn't be sufficient to use a normal variable here because executables are added several scopes down (and also in test/gtest/, which is not under src/). Maybe you have a larger refactoring in mind? I would strongly prefer to declare Mission Accomplished on this and not try to get a very different architecture working, given how difficult it is to make correct changes here.

[tlively]

Also, FWIW EMSCRIPTEN and MSVC are normal CMake variables. They are visible in child scopes, but mutations performed in those scopes would be done to copies of the variables and would not be visible back in the parent or in sibling scopes. (Yes this is awful.)

[kripken]

Moving the flags management one scope down wouldn't be sufficient to use a normal variable here

Why do we need a normal variable here? The goal of the PR IIUC is to apply custom flags only to 1P code. If we had a directory structure like this:

src/        ;; all of this is 1P code
thirdparty/ ;; all of this is 3P code

then wouldn't the problem be trivial? We would apply custom flags inside src/ and that's it. Sorry if I'm missing something obvious, but this seems simple to do (so likely I am confused).

[tlively]

Closing this in favor of #7241, which has landed.