Fix some warnings #3595
Fix some warnings #3595
Conversation
| @@ -170,7 +170,7 @@ namespace mamba | |||
| { | |||
| public: | |||
|
|
|||
| MessageLogger(const char* file, int line, log_level level); | |||
| MessageLogger(const char* file, log_level level); | |||
There was a problem hiding this comment.
Can you explain why remove the line here?
There was a problem hiding this comment.
m_line is not used anywhere, we're not logging the line number, so the compiler was warning that the class data member was unused.
There was a problem hiding this comment.
Hmm, ok. It seems to be the case for file as well.
No warning so far?
There was a problem hiding this comment.
It doesn't give a warning (now), but it's a good point, I will remove m_file as well - it is not used and doesn't change any behaviour.
| @@ -58,7 +58,8 @@ namespace mamba::download | |||
| TEST_CASE("PassThroughMirror") | |||
| { | |||
| std::unique_ptr<Mirror> mir = make_mirror(""); | |||
| CHECK_EQ(typeid(*mir), typeid(PassThroughMirror)); | |||
| auto& ref = *mir; | |||
There was a problem hiding this comment.
How about the warning for this change? I believe this is more related to performance but we only dereference once.
There was a problem hiding this comment.
This is not about performance, compiler didn't like that there possibly was a call with side effects in typeid call (dereferencing might fail).
The easiest way to fix this is to dereference in a separate line.
There was a problem hiding this comment.
Well in that case, it still doesn't solve the problem of dereferencing possible failure.
I suggest adding instead a static_assert before the CHECK_EQ:
static_assert(mir != nullptr);
There was a problem hiding this comment.
I can't do it, because mir is not known in a compile-time, so static_assert won't work here
No description provided.