snitch doesn't compile

[s9w]

context: 1.2.2 release, header-only mode, newest VS (17.8.0 preview 2)

Even compiling a trivial

#define SNITCH_IMPLEMENTATION
#include <snitch/snitch_all.hpp>

TEST_CASE("Factorials are computed", "[factorial]") {
   REQUIRE(true);
}

results in error C2397: conversion from 'long' to 'size_t' requires a narrowing conversion

[s9w]

I think I nailed it down to this line, specifically the __LINE__ macro. That might need a cast or something as it gets assigned to your std::size_t line member.

[cschreib]

Thanks for reporting this, and indeed that was my suspicion as well. A cast in the macro should fix it. Feel free to open a PR, otherwise I will do it tomorrow or this weekend.

NB there are a few places where this will be needed https://github.com/cschreib/snitch/blob/87260edd26c2005a7b8998130dc7e92fb15aef52/include/snitch/snitch_macros_test_case.hpp#L11 https://github.com/cschreib/snitch/blob/87260edd26c2005a7b8998130dc7e92fb15aef52/include/snitch/snitch_macros_test_case.hpp#L22 etc...

[cschreib]

A neater alternative would be to use std::source_location, but I was worried of the potential compilation time overhead. Perhaps unjustifiably so since I never measured it.

[s9w]

This was probably more or less the exact place for which std::source_location was intended, so I'd give it a go yeah.

[cschreib]

https://github.com/cschreib/snitch/pull/130/commits/a7d9e462ba30dcb4d638fda3eb4be35de34f74b4 should fix the problem, if you get a chance to test it.

I'm refraining from using std::source_location for now. Its main advantage is that it doesn't require macros, but since snitch checks are macro-based, this isn't something we really benefit from. It would just create overhead, since eventually we want to store a {std::string_view, std::size_t}, that it would need converting into. It will be more useful when reflection comes in and can decompose expressions without macros, then snitch can become mostly macro-free. But we're not there yet!

[s9w]

So many things will get easier with reflections :<. I'll test tomorrow, thanks.

On Sat, Sep 30, 2023, 11:43 Corentin Schreiber @.***> wrote:

a7d9e46 https://github.com/cschreib/snitch/commit/a7d9e462ba30dcb4d638fda3eb4be35de34f74b4 should fix the problem, if you get a chance to test it.

I'm refraining from using std::source_location for now. Its main advantage is that it doesn't require macros, but since snitch checks are macro-based, this isn't something we really benefit from. It would just create overhead, since eventually we want to store a {std::string_view, std::size_t}, that it would need converting into. It will be more useful when reflection comes in and can decompose expressions without macros, then snitch can become mostly macro-free. But we're not there yet!

— Reply to this email directly, view it on GitHub https://github.com/cschreib/snitch/issues/129#issuecomment-1741727200, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABODVHRHT3BAXNQK2GIBS3TX47SSPANCNFSM6AAAAAA5LNNMTM . You are receiving this because you authored the thread.Message ID: @.***>

[cschreib]

Merged and released; I assumed all went fine :)