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]

a7d9e46 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 <a7d9e46> 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 <#129 (comment)>, 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 :)