Conversation
115fa07 to
6ac78ff
Compare
rubidium42
left a comment
There was a problem hiding this comment.
I support the removal of boost.
| #define INTERNAL_ERROR(var, val) \ | ||
| do { \ | ||
| const std::source_location location = std::source_location::current(); \ | ||
| IssueMessage(0, INTERNAL_ERROR_TEXT, location.file_name(), location.line(), _spritenum, #var, val, location.function_name()); \ | ||
| assert(false); \ | ||
| exit(EFATAL); \ | ||
| } while (false) |
There was a problem hiding this comment.
Why not convert this into a [[noreturn]] function while you're at it? Something akin:
[[noreturn]] static inline void INTERNAL_ERROR(std::string_view var, const auto &val, const std::source_location location = std::source_location::current())
{
IssueMessage(0, INTERNAL_ERROR_TEXT, location.file_name(), location.line(), _spritenum, var, val, location.function_name());
assert(false);
exit(EFATAL);
}
There was a problem hiding this comment.
I did try that initially, but ran into the issue that var is not always a string and I wasn't willing to change all the callsites. That said doing so is probably smaller than all the other changes I ended up making
| if(!(cond))INTERNAL_ERROR(var,var);\ | ||
| else\ | ||
| ((void)0) | ||
| if(!(cond))INTERNAL_ERROR(#var,var) |
There was a problem hiding this comment.
This triggers my spidey-sense:
if (foo) VERIFY(bar, baz);
else std::println("foo: {}", foo);
Is going to print foo: true. I know this situation doesn't exist in the code, but it might become surprising at some point. The previous code did not open that hole.
| - static_cast<local_days>(SINCE_1920) | ||
| + static_cast<day>(extra)); |
There was a problem hiding this comment.
For what it's worth, I reckon extra is equivalent to either 0 or local_days(SINCE_1920). It would be more logical if extra became something like epoch, with 0 in the _D_ branch and SINCE_1920 in the _W_ branch. And then just subtracting epoch in this return.
I'm not going to claim it works, needs someone with far more knowledge of grfcodec to me to actually test it. But it does compile, so...
boost::for_eachwith actual ranged for loops and algorithmsboost::bimapsusage for NFO escapes withstd::vector<std:pair>boost::gregorianwithstd::chrono