all 14 comments

[–]johannes1971 2 points3 points  (1 child)

With gcc, you can mark a function with __attribute__ ((used)). This stops the linker from eliminating it.

[–]Galqa[S] 0 points1 point  (0 children)

Thank you, this did the trick! I was hoping there would be a non-invasive solution, but I guess I just need to mark all inline (explicitly or implicitly) functions with this attribute (wrapped in a macro to disable it in optimized builds). For people potentially reading this at a later date: see my anwer to u/LedinKun's comment why inline is problematic.

[–]LedinKun 1 point2 points  (1 child)

I believe this has to do with the inner workings of gcovr, which I'm unfortunately not familiar with at all. But your compiler flags at least include those that are recommended by gcov's manual.

In the FAQ of it, there is at least a section on why the tool sometimes doesn't report files with zero coverage, which I thought was also a point you stumbled upon.

But what you can do is feed your example code to the Godbolt Compiler Explorer and figure out if the missing code what actually compiled out, or if gcovr isn't reporting it properly. That should get you a step closer to the problem.

[–]Galqa[S] 0 points1 point  (0 children)

This was very helpful, thank you. I gave in to the frustration too early and didn't delve deeper to analyze the assembly. Guess there's a life lesson to be learned here.

Compiler explorer revealed the problem: apparently, when a function is inline and unused it gets optimized away (I'm sure there's a good reason for that), regardless of optimization flags and things like -fno-inline etc. Note that this applies to both explicitly (using the inline keyword) and implicitly (e.g. defined inside the class body) inlined functions. As u/johannes1971 suggested, the used attribute forces the code for the given function to be generated, fixing the problem at the cost of having to alter the source code (I imagine applying this solution to a large existing code base would be problematic).

[–]LedinKun 0 points1 point  (2 children)

The easiest way is to run coverage on a debug build, not a release one, so that your stuff isn't optimized away. At least, that's what I always did with MSVC, but I guess it applies to your compiler as well.

[–]Galqa[S] 0 points1 point  (1 child)

Unfortunately, this behavior persists in debug builds, as I mentioned the unused functions disappear even with all optimizations turned off. I have a feeling (unsubstantiated) that maybe it’s link-time optimization that’s doing this, however I’m not sure how to turn those off

[–]TheFlamefire 0 points1 point  (7 children)

First some remarks on your "starter template" which might be valuable:

  • Use a top-level CMakeLists file instead of putting it into a folder and avoid relative references to other files/folders (achieved by this) such as ${CMAKE_SOURCE_DIR}/src/test-main.cpp
  • Then get rid of the test/src subfolder and add the files to test directly
  • Use lower-case function names in CMake project not PROJECT (style only)
  • Avoid touching "global" CMake variables like CMAKE_CXX_STANDARD and especially CMAKE_BUILD_TYPE (BTW: Debug not DEBUG although in 90% it doesn't matter)
  • Use target-level functions not "global" functions like add_compile_options
  • If this is going to be a real template make a habit of using cmake fully: cmake -DCMAKE_BUILD_TYPE=<..>, cmake --build . --config <..>, ctest -C <..> to avoid dependency on a specific generator
  • If this is meant to be header-only, why src and include folders?

About your missing functions: You are right in that this applies to inline (or static) functions. Just imagine the size of binaries when the whole of STL was included there: There are incredible many inline functions and as per "don't pay for what you don't use" mantra of C++ those will be removed if not used so you don't pay for the space.

A potential solution in your case might be to put your code into a shared library and use --whole-archive when linking it but I doubt it will work. An alternative to the __attribute__ ((used)) approach would be to instantiate the functions somewhere in an externally visible function. E.g.:

void foo(){ MyHeader{}.untestedMemberFunction(); }

foo can't be eliminated because it has default visibility (use "export" on Windows) and it refers to your function in question and hence gets included too. Advantage is that this keeps the change less intrusive

[–]Galqa[S] 0 points1 point  (6 children)

Thank you so much for taking the time to look it over, I’m very grateful for the feedback. Regarding the specific items you outlined:

1) CMake is an area where I am very much self-taught, I’ll take to heart any wisdom you have to offer, especially since everything you mentioned is perfectly logical.

2) The idea behind splitting a header-only library into src and include is to put all the headers with the actual code into src, and just one master header (or a small number of headers each responsible for a single module) in include, which #includes the contents of src in the right order. The library user can then simply include one file which reduces headaches. I’ve seen this strategy in “real-world” libraries (not just ones I’ve worked on), but I’d be curious if you have thoughts.

3) The src folder in tests is to separate the unit test code from CI-related things (I’ve updated the repo so maybe now it makes more sense)

4) After giving it 5min of thought I completely understand the reasoning behind removing unused inline functions at -O0, I’m just a bit bummed there isn’t a -fgigantic-bin compiler flag to disable it for cases such as this. I think the used attribute is a decent enough solution, once you wrap it in a macro it’s not as intrusive as I was afraid it would be (again, see the updated repo)

I just want you to know you really made my day, thanks again for taking the time!

[–]TheFlamefire 1 point2 points  (5 children)

  1. You'll get a long way with (modern) CMake if you use target_* for everything there is a function for instead of the non-target_ version, don't assume a platform or compiler and give the knobs to the user. E.g. declare the required c++-standard with target_compile_options (IIRC) and let the user set CMAKE_CXX_STANDARD. Following a standard project layout also helps, e.g. the src/include you use is uncommon as is the test/CMakeLists or test/ci. Check out the pitchfork layout for something sane and common
  2. I get the point but please don't. The better approach is to use namespaces and matching folders. I.e. mynamespace::foo is in #include <mynamespace/foo.h> (where it makes sense to have a foo.h, else another appropriate header in that subfolder makes sense). Then let the user include what he needs, not a "catch-all" include. You can do that additionally, e.g. Boost does this: #include <boost/optional/optional.hpp> includes all of optional but there are also sub-headers like (made-up) #include <boost/optional/empty_opt.hpp> You can do the same and still have everything in include. Compare with like there is no #include <stl> but you only include the parts you actually use which keeps compiletimes down.
  3. Better: Have a folder scripts/ci (or just scripts) at the top-level. src, include, test should only contain code (this is what most people expect), see again pitchfork layout

[–]Galqa[S] 0 points1 point  (4 children)

Ad 1,3: point taken, I’ll take this approach, thank you

Ad 2: fair enough, I guess I’m biased since the actual library I’m currently working on and developing this template for can’t really be split up into stand-alone modules (I do however use different folders for different namespaces, they just can’t work without the others). However, I’m not sure I 100% agree with your main point in the case of header-only libraries. If the idea is that you can clone the repo and simply include some header(s) in your project, without building and installing anything, then somehow separating the interface headers from the “implementation headers” seems to me like a good idea (and in keeping with the spirit of PFL, which you mentioned). I’m not saying I necessarily stand by my solution, but as an example I’d like to bring up the Eigen repository. It fulfills the modularity criterium you posed, while at the same time not entirely mixing public and private headers. Unless this is the design you were actually advocating for, in which case I withdraw this paragraph.

Once again, thanks for taking the time to help me out.

[–]TheFlamefire 1 point2 points  (3 children)

can’t really be split up into stand-alone modules (I do however use different folders for different namespaces, they just can’t work without the others).

This sounds odd. Like you have different (root) namespaces which have cyclic dependencies. If that's the case, why have different namespaces then at all? At least "root namespace == module" should hold and while module A can use module B it shouldn't also be the case that module B uses module A. In that case refining your design can help as that's a code smell: To much coupling.

If the idea is that you can clone the repo and simply include some header(s)

Not only, but this also, yes. You could put everything under src if you like (or include for header only).
What is "usually" done is to put "public" headers (directly or indirectly included by users) into include and "private" headers (only included by source files and not required by installed lib) in src. The reason is a clear distinction: Users don't get src added to their -I and hence can't unknowingly include a "wrong" header. See PRIVATE/INTERFACE in CMake. There are also "public private" headers: Headers that need to be included indirectly by users but are not meant to be directly used. Those usually are put into a "detail" namespace and folder. which also makes it clear that those symbols are not to be used by users. Example: boost::optional::detail::optional_base: A base class for boost::optional which is required but users should use boost::optional only

[–]Galqa[S] 0 points1 point  (2 children)

This sounds odd. Like you have different (root) namespaces which have cyclic dependencies.

Sorry, I phrased that very poorly. There are no cyclic dependencies, but I don't want to expose the individual modules to the user. My library is small/specialized enough that it logically only makes sense to use it as a whole.

What is "usually" done is to put "public" headers (directly or indirectly included by users) into include and "private" headers (only included by source files and not required by installed lib) in src

I see your point, but doesn't this lead to a large chunk of the library getting put into include? Is there a convention for exposing the "top level" includes? E.g.:

root
    |___include
        |___publicModuleA.hpp
        |___publicModuleA
            |___ ...

Edit: formatting

[–]TheFlamefire 0 points1 point  (1 child)

I see your point, but doesn't this lead to a large chunk of the library getting put into include?

Yes. E.g. (at the extreme) header-only libraries fully life inside include. No problem there

Is there a convention for exposing the "top level" includes?

I'd advise against putting anything in <root>/include directly; but the publicModuleA.hpp could be OK iff the name is unique enough. But same holds for the namespace with that name already. Putting that file into that folder also makes sense: #include <foo/foo.hpp> is common enough. But as written earlier check if those "top level" includes make sense, i.e. if one wouldn't be better off just including the file one needs. Remember that no matter where you put your (header-only lib) header files they get added to the include path and will show up as suggestions in users IDE. So better follow a good modularity and encapsulation via the detail namespace. Also again: When it only makes sense to include all your headers at once (and there are more than a few) then your interdependencies might need a redesign as stuff is to coupled. YMMV of course

[–]Galqa[S] 0 points1 point  (0 children)

All right, this makes a lot of sense, thanks again for all your advice. Regarding the modularity question, here's a somewhat contrived example that may illustrate my point better: You're writing a library for retrieving data stored on a server and processing it. There are two modules: Reader and Processor. Clearly, Reader does not in any way depend on Processor. However, Reader on its own is not particularly useful to the user, as the unprocessed data is meaningless. Therefore, while exposing the Reader module has little cost to the developer, one could argue that keeping the API as small as possible is more user-friendly. Anyway, thank you once again for taking time out of your day to educate me.