Compile options and warnings for standalone?

[friendlyanon]

Similar to how it's done in this other starter attempt, I feel like those options and warnings could be beneficial.

I would like to get some input on how the IPO (LTO) optimization should be done before making a PR.
In that other project it seems to be a global option whether LTO is on or off. Is that right?

[TheLartians]

Yeah, as LTO affects the linking it's only actually useful when building executables, so it makes sense to set it as a global option to affect all executables.

As LTO support is already built in to CMake I don't think we need to add any code to the starter. Users can simply configure the build using cmake -Hstandalone -Bbuild/standalone -D CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON or set the value directly in the standalone/CMakeLists.txt: set(CMAKE_INTERPROCEDURAL_OPTIMIZATION ON).

TBH, I'm a bit unsure how I feel about adding warnings in the standalone, as warnings are sensitive to the current compiler version and can break the build for non-developer users. On the other hand I do see the value during development. Maybe it makes sense to add the warnings to the standalone from the all subproject, as that is where library developers will usually be working.

[friendlyanon]

Right now I'm toying around with this starter, and the way I do things now is have a scripts folder with the configure and build steps.

For example, this is my script for release build:

```sh #!/bin/sh CWD="$(cd "$(dirname "$0")" && pwd)" cd $CWD/.. configure() { cmake -Hstandalone -Bbuild/release -GNinja \ -DCMAKE_BUILD_TYPE=Release \ -DUSE_CCACHE=ON \ -DCPM_SOURCE_CACHE="$(pwd)/.cpm-cache" } build() { cmake --build build/release --config Release } configure && build ```

I realise sh scripts alone are not a cross-platform solution, but if there were scripts like this then there would be a way to restrict warnings to development only builds in an intuitive way.

[TheLartians]

I actually have something similar set up for my projects, but usually use Python instead of sh. :) As this is a more advanced feature and the use-cases and preferences vary a lot I think it may be best to leave this for the users to implement.

Edit: here's another example of me using package.json for configuring web projects.

[friendlyanon]

This is quite the conundrum indeed.

I believe those warnings would provide really helpful diagnostics for people who might want to start learning C++ using this project, which - from the looks of it - is an intended goal.

[TheLartians]

For the warnings part I agree, though I don't think adding the complexity of an extra build script is best approach. But perhaps instead we could just add the warnings to the standalone from all/CMakeLists.txt as suggested above.

As for the intended goal of the project ... I personally use this starter for all my new C++ projects as it saves a lot of time setting things up and wouldn't exactly consider myself a beginner. 😄

[friendlyanon]

While you might not consider it one, I believe that this is a very good project that can only benefit someone looking to learn.
Finding concrete examples of dos and don'ts for "modern" CMake with snippets that don't brush over a lot of context is nigh impossible.

[TheLartians]

Hehe thanks! To be clear, I definitely also think that this starter is a great idea for a beginners as well, as it enforces many good practices by design. :) CMake has a hell of a learning curve and most resources are either outdated or too vague, just as you say.

[friendlyanon]

So I have been toying around with this thing and it appears that warnings would only work if 3rd parties were included as SYSTEM libraries. See https://github.com/friendlyanon/ModernCppStarter/commit/31432bc93d0ca8853ffa9182c3261fb1412b1ac4

However, this brings up another question. Is it correct that libraries included using CPM aren't linked to as SYSTEM libraries?

[friendlyanon]

Would this commit sufficiently cover this issue? https://github.com/friendlyanon/ModernCppStarter/commit/303fe4c47259512107018e8ae59f9f54e8a457f3

I don't know how find_package would bring in libraries, so I'm not sure about how universal this solution would be.

[TheLartians]

I suppose it probably solve the issue of imported header warnings for users of your library, although it's very unusual for C++ projects to implement such an option. Also after thinking about it, I personally prefer to get compiler warnings in imported headers, as they can show legitimate issues, such as preprocessor misconfigurations or simply act as a red flag against using the library.

Another problem I found is that the specification of SYSTEM is not completely clear may and using it for third-party libraries may have unintended consequences. From the CMake docs:

If SYSTEM is specified, the compiler will be told the directories are meant as system include directories on some platforms (signalling this setting might achieve effects such as the compiler skipping warnings, or these fixed-install system files not being considered in dependency calculations - see compiler docs). If SYSTEM is used together with PUBLIC or INTERFACE, the INTERFACE_SYSTEM_INCLUDE_DIRECTORIES target property will be populated with the specified directories.

[friendlyanon]

In a perfect world, no library would contain code that would emit warnings, but alas, here we are.

It also seems weird to me that I have to care about other people's code to this degree.
In other languages static analysis can only apply to my own code, because that is the only code I have full control over.
I think what you described can be handled by https://github.com/friendlyanon/ModernCppStarter/commit/849fd52483d4d78435ca7d21c6098b9918b83084 well enough.

I asked in the #cmake IRC channel and noone seemed troubled that I would want to use SYSTEM includes and were instead told that in fact IMPORTED targets already act as if they were SYSTEM.

[TheLartians]

That's one of the perks of using a language who's dependency management mechanism consists of essentially copy-pasting third-party code into your source. The warnings are actually happening inside your code and not the third-party library.

IMHO your solution is certainly a valid way of dealing with it and can make it convenient for users of your library if your headers are generating warnings. Personally, I prefer to be notified of warnings in third-party headers, as they can make me aware of depreciated functionality or missing / wrong macros defined in my code.

I don't think there's any great solution to this besides switching to modules, whenever they might be available.

[friendlyanon]

Hmm, true. So it seems there is no general solution to this as it stands now.

I think this topic can be closed for now and maybe revisited once tooling around modules becomes universal.

[oddlama]

I just wanted to point out that if your goal is just to disable warnings in included headers (without the downsides of using SYSTEM), you can do so by creating a wrapper header. Modules are definitely the way to go for the future, but until then you can do the following (example uses fmtlib):

include/myproject/external/fmt.hpp:

#pragma once

// Supress warnings in external libraries
#pragma GCC system_header
#pragma clang system_header

#include <fmt/format.h>

You can then use #include <myproject/external/fmt.hpp> when you need the library.

A wrapper header also comes with the added benefit that you have a single location where you can customize your dependency. For example if you wanted to use libfmt as a header only library, you need to #define FMT_HEADER_ONLY before every include, which is prone to errors if you use the library in a lot of different files. Here you can just stick it in the wrapper.

A downside is that you can't easily include a subset of a big library this way. Yet to me this approach always felt very organized, and until we get modules I guess it might be a good compromise.

[friendlyanon]

For example if you wanted to use libfmt as a header only library, you need to #define FMT_HEADER_ONLY before every include, which is prone to errors if you use the library in a lot of different files. Here you can just stick it in the wrapper.

The fmt::fmt-header-only target is there for that, you don't need to do anything other than link against it.

If you look at my recent GitHub activity, you can see that the only way to have the same experience for dependents - whether they consume a library using CMake packages or making it a part of their build tree - is to have a warning guard in the root lists file. See here.
In the case of pulling the library into the build tree, the dependent has the option to set a variable in the form of set("${library_name}_INCLUDE_WITHOUT_SYSTEM" YES CACHE BOOL "") to enable warnings.

This way the dependent gets the exact same experience, no matter how they pull in a library.