Scopes do not work properly on windows.

[edmiester777]

I use the example code on windows (minor tweaks to compile):

And here is what outputs:

It appears that the LOG_SCOPE_F macro immediately closes the scope while on windows.

Please note that I am using VS2017 to build.

Edit: It also appears that any scope immediately closes in this build.

[emilk]

Very weird. I don't have any access to Windows, so this is hard for me to debug. Anybody like to take a stab at it?

[edmiester777]

I'll take a crack at it.

[tomaszte]

Hi, got the same problem on windows. Done some tests:

//destructor called immediately
LOG_SCOPE_F(INFO, "TEST.");

//destructor called immediately
loguru::LogScopeRAII error_context_RAII_TEST = ((loguru::Verbosity_INFO) > loguru::current_verbosity_cutoff()) ? loguru::LogScopeRAII() : loguru::LogScopeRAII(loguru::Verbosity_INFO, "win32_editor.cpp", 519, "TEST.");

//destructor not called, indendation works fine
loguru::LogScopeRAII error_context_RAII_TEST = loguru::LogScopeRAII(loguru::Verbosity_INFO, "win32_editor.cpp", 519, "TEST.");

So I guess windows treats it like that (expression) ? {/*scope1*/ value1 } : {/*scope2*/ value2 }; Any idea for a fix?

[paralaxsd]

Hi!

I've come across the same issue and have a solution that works for me. However, I'm currently using VS 2013 which brings its own problems. I'll suggest fixes for both areas. Your mileage may vary with newer VS versions since my symptoms are not exactly the same but I guess this may help anyhow. Let me elaborate:

1.) VS 2013 has issues with implicitly generated move constructors and does not understand the noexcept keyword. We can handle these issues in two steps:

• Add This code to the end of the conditional block for _MSC_VER:

#if (_MSC_VER <= 1800)
#include <xkeycheck.h>
#include <string.h>
#define noexcept(A)
#endif

This fixes the issues with noexcept and includes string.h because we want to strcpy_s the _name buffer later.

• In the LogScopeRAII class, replace LogScopeRAII(LogScopeRAII&& other) = default; with

#if _MSC_VER >1800
		LogScopeRAII(LogScopeRAII&& other) = default;
#else 
    LogScopeRAII(LogScopeRAII&& other) :
      _verbosity(other._verbosity), _file(other._file), _line(other._line), _indent_stderr(other._indent_stderr),
      _start_time_ns(other._start_time_ns)
	  {
      strcpy_s(_name, other._name);
      other._file = nullptr;
    }
#endif

This also fixes the scoping issue: originally, when the right hand side object that's being assigned in the macro goes out of scope, its _file pointer is still valid and produces an additional log call. Setting the pointer to nullptr prevents the destructor from logging anything and changing the indentation level.

2.) Bonus: I've also added this code into the general _MSC_VER block:

#include <stdio.h>
#define snprintf _snprintf

Otherwise VS complains that it doesn't know snprintf. Also, the header can't be included all by its own otherwise.

[dbremner]

@paralaxsd, _snprintf does not guarantee null termination. Here is a quote from the documentation. The _snprintf family of functions only appends a terminating null character if the formatted string length is strictly less than count characters. Here is a self contained replacement that might work for Visual Studio 2013.

[alliepiper]

@emilk I have a patch that fixes this on MSVC2015 x64. Implementing the move constructor and explicitly clearing the rvalue object's _file does the trick.

Just include <utility> for std::move and change the defaulted move constructor to:

    LogScopeRAII(LogScopeRAII&& other)
    {
      _verbosity = std::move(other._verbosity);
      _file = std::move(other._file);
      _line = std::move(other._line);
      _indent_stderr = std::move(other._indent_stderr);
      _start_time_ns = std::move(other._start_time_ns);
      memcpy(_name, other._name, LOGURU_SCOPE_TEXT_SIZE * sizeof(char));

      // Make sure the tmp object's destruction doesn't close the scope:
      other._file = nullptr;
    }

If you don't want to include the additional header, just copying instead of moving shouldn't be much of an issue in this case.

[emilk]

@allisonvacanti I appreciate your PR, but I absolutely want to avoid #includes in the Loguru header. Luckily there should be no need: all the fields can just be copied, removing the need for std::move. _name can be copied with a small for-loop, removing the need for memcpy (and thus <cstring>). I suggest you change your PR along these lines. Maybe we should also only define LogScopeRAII(LogScopeRAII&& other) for the compilers that need it (MSVC) to avoid potential overhead on other platforms?

[alliepiper]

Agreed -- I'd actually fixed this before seeing this PR, and @paralaxsd's solution is better, since we can avoid std::move using the initialization syntax (for some reason I wrote mine like an move assignment operator instead of a ctor).

I'll update the PR with your suggested changes and incorporate the earlier patch too.