gabime / spdlog

Fast C++ logging library.
Other
24.55k stars 4.58k forks source link

cannot bind non-const lvalue reference of type ‘string&’ to an rvalue of type ‘string’ #3116

Open Iuliean opened 5 months ago

Iuliean commented 5 months ago

I keep getting this error when building with cmake and i cant figure out what is happening

make[2]: Entering directory '/home/iulian/dev/minecraft-server/build'
[ 29%] Building CXX object dependencies/SFW/src/CMakeFiles/SFW.dir/Connection.cpp.o
cd /home/iulian/dev/minecraft-server/build/dependencies/SFW/src && /usr/bin/g++ -DSFW_EXPORTS -DSPDLOG_COMPILED_LIB -DSPDLOG_USE_STD_FORMAT -I/home/iulian/dev/minecraft-server/dependencies/SFW/include/SFW -I/home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include -g -std=gnu++20 -fPIC -MD -MT dependencies/SFW/src/CMakeFiles/SFW.dir/Connection.cpp.o -MF CMakeFiles/SFW.dir/Connection.cpp.o.d -o CMakeFiles/SFW.dir/Connection.cpp.o -c /home/iulian/dev/minecraft-server/dependencies/SFW/src/Connection.cpp
In file included from /home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/spdlog.h:14,
                 from /home/iulian/dev/minecraft-server/dependencies/SFW/include/SFW/Logger.h:3,
                 from /home/iulian/dev/minecraft-server/dependencies/SFW/include/SFW/Connection.h:12,
                 from /home/iulian/dev/minecraft-server/dependencies/SFW/src/Connection.cpp:6:
/home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/logger.h: In instantiation of ‘void spdlog::logger::log_(spdlog::source_loc, spdlog::level::level_enum, spdlog::string_view_t, Args&& ...) [with Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; spdlog::string_view_t = std::basic_string_view<char>]’:
/home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/logger.h:90:13:   required from ‘void spdlog::logger::log(spdlog::source_loc, spdlog::level::level_enum, spdlog::format_string_t<Args ...>, Args&& ...) [with Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; spdlog::format_string_t<Args ...> = std::basic_string_view<char>]’
/home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/logger.h:96:12:   required from ‘void spdlog::logger::log(spdlog::level::level_enum, spdlog::format_string_t<Args ...>, Args&& ...) [with Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; spdlog::format_string_t<Args ...> = std::basic_string_view<char>]’
/home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/logger.h:170:12:   required from ‘void spdlog::logger::error(spdlog::format_string_t<Args ...>, Args&& ...) [with Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; spdlog::format_string_t<Args ...> = std::basic_string_view<char>]’
/home/iulian/dev/minecraft-server/dependencies/SFW/include/SFW/Logger.h:49:28:   required from ‘void iu::Logger::error(spdlog::format_string_t<Args ...>, Args&& ...) const [with Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; spdlog::format_string_t<Args ...> = std::basic_string_view<char>]’
/home/iulian/dev/minecraft-server/dependencies/SFW/include/SFW/Connection.h:83:31:   required from here
/home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/logger.h:372:88: error: cannot bind non-const lvalue reference of type ‘std::__cxx11::basic_string<char>&’ to an rvalue of type ‘std::__cxx11::basic_string<char>’
  372 |             fmt_lib::vformat_to(std::back_inserter(buf), fmt, fmt_lib::make_format_args(std::forward<Args>(args)...));
      |                                                               ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/13/bits/chrono_io.h:39,
                 from /usr/include/c++/13/chrono:3370,
                 from /home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/common.h:10,
                 from /home/iulian/dev/minecraft-server/dependencies/SFW/dependencies/spdlog/include/spdlog/spdlog.h:12:
/usr/include/c++/13/format:3411:28: note:   initializing argument 1 of ‘auto std::make_format_args(_Args& ...) [with _Context = basic_format_context<__format::_Sink_iter<char>, char>; _Args = {__cxx11::basic_string<char, char_traits<char>, allocator<char> >}]’
 3411 |     make_format_args(_Args&... __fmt_args) noexcept
      |                      ~~~~~~^~~~~~~~~~~~~~
make[2]: *** [dependencies/SFW/src/CMakeFiles/SFW.dir/build.make:76: dependencies/SFW/src/CMakeFiles/SFW.dir/Connection.cpp.o] Error 1
make[2]: Leaving directory '/home/iulian/dev/minecraft-server/build'
make[1]: *** [CMakeFiles/Makefile2:195: dependencies/SFW/src/CMakeFiles/SFW.dir/all] Error 2
make[1]: Leaving directory '/home/iulian/dev/minecraft-server/build'
make: *** [Makefile:91: all] Error 2

My project structure goes like this. image

And these are the cmake files.

cmake_minimum_required(VERSION 3.27.7)

project(SFW VERSION 0.1)

include(ExternalProject)

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED True)
set(SPDLOG_USE_STD_FORMAT ON)

add_subdirectory(dependencies/spdlog)
add_subdirectory(src)

add_library(SFW_EXPORT INTERFACE)
add_library(SFW::SFW ALIAS SFW_EXPORT)

target_include_directories(SFW_EXPORT
    INTERFACE ${PROJECT_SOURCE_DIR}/include)

target_compile_features(SFW_EXPORT
    INTERFACE cxx_std_20)

target_compile_definitions(SFW_EXPORT
    INTERFACE SPDLOG_USE_STD_FORMAT=1)

target_link_libraries(SFW_EXPORT
    INTERFACE spdlog::spdlog_header_only
    INTERFACE SFW)
add_library(${PROJECT_NAME} SHARED
    Connection.cpp
    Logger.cpp
    LoggerManager.cpp
    Server.cpp
    ServerConnectionHandler.cpp
    Socket.cpp
    SocketDescriptor.cpp
    ThreadPool.cpp
    utils.cpp)

#includes main.cpp for when i want to test stuff 
add_executable(${PROJECT_NAME}-test
    main.cpp
)

target_include_directories(${PROJECT_NAME}
    PRIVATE     ${PROJECT_SOURCE_DIR}/include/SFW)

target_link_libraries(${PROJECT_NAME} PRIVATE spdlog::spdlog_header_only)

target_include_directories(${PROJECT_NAME}-test
    PRIVATE ${PROJECT_SOURCE_DIR}/include/SFW)

target_link_libraries(${PROJECT_NAME}-test
    PRIVATE ${PROJECT_NAME})
cmake_minimum_required(VERSION 3.27.7)

project(mc-server)

set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_SOURCE_DIR}/out/$<CONFIG>/)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_SOURCE_DIR}/out/$<CONFIG>/)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

add_subdirectory(dependencies/SFW)
add_subdirectory(dependencies/nlohmann-json)

add_subdirectory(src)

target_include_directories(${PROJECT_NAME}
                        PRIVATE include/
                        PRIVATE dependencies/nlohmann-json/single_include)

target_link_libraries(${PROJECT_NAME} PRIVATE SFW::SFW)

set_property(TARGET SFW PROPERTY CXX_STANDARD 20)
add_executable(${PROJECT_NAME}
    main.cpp
    MinecraftHandler.cpp
    PlayerHandler.cpp
    ServerPackets.cpp
    utils.cpp
    DataTypes/Identifier.cpp
    DataTypes/nbt.cpp)

I hope i am not completely misusing cmake though

tt4g commented 5 months ago

The compile error cannot bind non-const lvalue reference of type ‘std::__cxx11::basic_string<char>&’ to an rvalue of type ‘std::__cxx11::basic_string<char>’ is caused by your source code passing rvalue where lvalue is needed. This is not a problem with your use of CMake. And we cannot help you if you do not provide source code to the source of the error.

tt4g commented 5 months ago

I found a code that may be the cause of the error.

https://github.com/Iuliean/SFW/blob/3a23d302d001bf54445bbc606b16a6a2548c8320/include/SFW/Connection.h#L77-L87

It appears to be a known problem (specification): fmtlib/fmt#3596

             if(received == -1)
             {
-               m_logger.error("Failed to receive data: {}", utils::getErrorFromErrno());
+               std::string err = utils::getErrorFromErrno();
+               m_logger.error("Failed to receive data: {}", err);
                 exit(1);
             }
Iuliean commented 5 months ago

Hmm interesting this worked untill recently for me, that's why i kinda dismissed the reference error. I thought i might have screwed something up when changing the cmake files.

Also i dont see where this issue is mentioned

tt4g commented 5 months ago

It appears to me that this change has caused the return value of utils::getErrorFromErrno() to be recognized as the format argument by fmt library.

And since the changed code is written in a header file, it is possible that the compiler did not build the code it deemed unreachable. Could it be that the compiler evaluated the changed code for the first time in an application that uses that library, resulting in a compile error?

Iuliean commented 5 months ago

If i understood correctly what you said the yes that is a possibility. but you can find plenty of examples throughout the code where i do the same thing but not in a header file and it did not happen.

https://github.com/Iuliean/SFW/blob/2d1bf050fcb98ea8feb94295edec91d43ee85f95/src/Connection.cpp#L26

This has pretty much always been there but no issues before.

My assumption is that the cmake files are fucked somehow since when i build the code with the above mentioned cmake files and project structure it also tries to tries to build the spdlog cpp files.

Also even weirder is that for me the change you mentioned works, it builds fine. if i revet back to the old one then yes it starts to fail again

As soon as i add it in the context of the other app mc-server it starts to fail again. It seems defines are not passed through i think includes are not there even though they should be i think

tt4g commented 5 months ago

The compiler may change the scope of optimization depending on the compile-time options and the code that could be recognized on a per-translation basis. I don't know what changes you have made to CMakeList.txt, but if the compiler's optimization behavior has changed, compile errors can occur that appear to be contrary to human intuition. Such problems are not under the jurisdiction of spdlog, but are either compiler specifications or bugs.

I can only suggest that if the change from rvalue to lvalue does not cause compile errors, then apply that change to all. That is the only way I know of to avoid the problem.

EDIT: Sometimes a non-standard feature causes the compiler to treat an rvalue as an lvalue, as in the following question: https://stackoverflow.com/questions/11508607/rvalue-to-lvalue-conversion-visual-studio This kind of compiler behavior is not something that can be managed by spdlog.

tt4g commented 5 months ago

I found fmtlib/fmt#3589 that clearly explains the fmt changes related to this issue.