implement SNITCH_DISABLE feature

[NicoMuleo]

Implement SNITCH_DISABLE build-time option to disable Snitch at build time.
This carries the benefit of removing the snitch symbols from the executable, especially when LTO is enabled.

change snitch disable configuration template

modified:   include/snitch/snitch_config.hpp.config

disable public API macros (if snitch is disabled). Add snitch::is_disabled constexpr variable.

modified:   include/snitch/snitch_macros_check.hpp
modified:   include/snitch/snitch_macros_check_base.hpp
modified:   include/snitch/snitch_macros_consteval.hpp
modified:   include/snitch/snitch_macros_constexpr.hpp
modified:   include/snitch/snitch_macros_exceptions.hpp
modified:   include/snitch/snitch_macros_misc.hpp
modified:   include/snitch/snitch_macros_reporter.hpp
modified:   include/snitch/snitch_macros_test_case.hpp
modified:   include/snitch/snitch_macros_utility.hpp

provide empty class 'registry' declaration and implementation

modified:   include/snitch/snitch_registry.hpp
modified:   src/snitch_registry.cpp

added option for disabling snitch

modified:   CMakeLists.txt

disable CLI functions

modified:   src/snitch_cli.cpp

added maybe_unused in append helper functions declaration

modified:   tests/runtime_tests/check.cpp

README update

[gaspacio]

The current tests do pass the build stage. The execution of tests of Snitch tested by Snitch itself doesn't have any effect. This seems to be the intended behavior.

[codecov]

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 94.01%. Comparing base (fe9249d) to head (ddf1002).
Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
src/snitch_main.cpp	42.85%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
- Coverage   94.17%   94.01%   -0.16%     
==========================================
  Files          30       30              
  Lines        1750     1754       +4     
==========================================
+ Hits         1648     1649       +1     
- Misses        102      105       +3     

Files with missing lines	Coverage Δ
include/snitch/snitch_matcher.hpp	100.00% <ø> (ø)
include/snitch/snitch_registry.hpp	86.36% <ø> (ø)
src/snitch_registry.cpp	95.58% <ø> (ø)
src/snitch_reporter_catch2_xml.cpp	95.45% <ø> (ø)
src/snitch_main.cpp	50.00% <42.85%> (+50.00%)	⬆️

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe9249d...ddf1002. Read the comment docs.

[cschreib]

Thank you very much for contributing this feature! I have some comments, mostly cosmetic, but also a few places where I think we can simplify things and make it easier to maintain longer term.

There are also a couple things missing:

• adding the new compile-time option to the Meson build scripts
• adding a CI run with snitch disabled, so we regularly test this new functionality

Unless you're keen to do this yourself, I can add these later, and for now we can focus first on resolving the open comments.

[NicoMuleo]

We added "disabled" jobs in cmake CI, but not in mason CI as we do not know how to do it.
"disabled" jobs skip all tests. Windows "disabled" job fails on setting stage, because of an "internal compiler error".

To give you more context about our use cases, we build snitch for embedded devices such as STM32 where it is required to squeeze the binary and the memory usage as much as possible.

With the latest commit we still have a double implementation of the registry, because it allows us to build a program like the one below (simplified) without needs of explicitly checking if snitch is disabled or not with the use of macro branches in the source code (in the spirit of doctest):

#define SNITCH_IMPLEMENTATION
#include <snitch/snitch_all.hpp>
 
struct Hardware {
  void check(int n) {
    while (n-- > 0) {
      INFO("check device numner #", n);
      CHECK(n < 15);
    }
  }
};
 
TEST_CASE("simple test case", "[simple]") {
  Hardware hw;
  hw.check(10);
}
 
int main(int argc, char **argv) {
  auto args = snitch::cli::parse_arguments(argc, argv);
  if (args) {
    snitch::tests.configure(*args);
    if (!snitch::tests.run_tests(*args))
      return true;
  }
  /*< Rest of the program...>*/
  return {};
}

[cschreib]

See my other comment above (in reply to the earlier comment on the registry code duplication); I think in this case we could simply require that you wrap this code in an if constexpr() to disable it:

int main(int argc, char **argv) {
  if constexpr (snitch::is_enabled) {
    auto args = snitch::cli::parse_arguments(argc, argv);
    if (args) {
      snitch::tests.configure(*args);
      if (!snitch::tests.run_tests(*args))
        return true;
    }
  }
  /*< Rest of the program...>*/
  return {};
}

That seems like a reasonable compromise to me, since presumably you don't need to touch the snitch C++ API anywhere else. I must say I am not keen on the alternative, where we have have to maintain two parallel versions of the C++ API with snitch enabled and disabled.

Another thing we could do to make this easier is to define a new macro SNITCH_MAIN(argc, argv), which normally executes the snitch main() function, but does nothing when SNITCH_ENABLE=OFF. Then you don't have to write any conditionals in your own code, unless you need to do custom stuff with the snitch C++ API (but then I think it's reasonable to expect that this custom code needs to be wrapped in a conditional).

[NicoMuleo]

I am answering here to all the comment up to today:
Understood, we will implement your suggestions as soon as possible.

[NicoMuleo]

Now everything should be as you asked, please have a look.
Please let us know if we need to do anything else.

[cschreib]

This new snitch::main() isn't being advertised in any header, so people can't use it unless they use the header-only build. I've added a header for it and moved it out of the SNITCH_DEFINE_MAIN block. Let me know if that wasn't what you intended.

[cschreib]

Thank you for pushing these changes! I took the liberty to push a few clean up commits, but as far as I can tell (and pending the CI runs OK) the PR is ready to go.

[NicoMuleo]

We like your changes, thank you very much for your help, we can't wait for the patch merge!