Dobiasd / FunctionalPlus

Functional Programming Library for C++. Write concise and readable C++ code.
http://www.editgym.com/fplus-api-search/
Boost Software License 1.0
2.1k stars 167 forks source link

Cannot compile with gcc 11.2 in C++20 mode #251

Closed tbreslein closed 2 years ago

tbreslein commented 2 years ago

Hi, I just found a weird "bug" that makes it so you cannot compile fplus (0.2.16) with gcc 11.2 in C++20 mode (maybe earlier compiler version work flawlessly, cannot test that right now unfortunately). It works perfectly fine with other C++ standards, and it works with clang (13.0). I have sorta "fixed" the issue though on my end, and that fix works with those two compilers on all standards 14-20.

Here's my CMake file (sorry for the noise with conan):

project(Foo LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 20)

# setup conan.cmake if it does not exist
list(APPEND CMAKE_MODULE_PATH ${CMAKE_BINARY_DIR})
list(APPEND CMAKE_PREFIX_PATH ${CMAKE_BINARY_DIR})

if(NOT EXISTS "${CMAKE_BINARY_DIR}/conan.cmake")
  message(STATUS "Downloading conan.cmake from https://github.com/conan-io/cmake-conan")
  file(DOWNLOAD "https://raw.githubusercontent.com/conan-io/cmake-conan/v0.16.1/conan.cmake"
                "${CMAKE_BINARY_DIR}/conan.cmake"
                EXPECTED_HASH SHA256=396e16d0f5eabdc6a14afddbcfff62a54a7ee75c6da23f32f7a31bc85db23484
                TLS_VERIFY ON)
endif()

include(${CMAKE_BINARY_DIR}/conan.cmake)

conan_cmake_configure(
    REQUIRES
        functionalplus/0.2.16
    GENERATORS
        cmake_paths
        cmake_find_package
    )

conan_cmake_autodetect(settings)

# download deps in conanfile.txt
conan_cmake_install(PATH_OR_REFERENCE .
    BUILD missing
    REMOTE conancenter
    SETTINGS ${settings}
    )

# add paths of conan managed packages
include(${CMAKE_BINARY_DIR}/conan_paths.cmake)

find_package(FunctionalPlus REQUIRED)

add_executable(main main.cpp)
target_link_libraries(main FunctionalPlus::fplus)

And I'm just configuring with:

cmake . -Bbuild -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER=<which ever compiler I'm testing>

Here's the source file for my little example (which is just examples from your README copy pasted lol):

#include <fplus/fplus.hpp>
#include <iostream>

struct cat {
    double cuteness() const
    {
        return softness_ * temperature_ * roundness_ * fur_amount_ - size_;
    }
    std::string name_;
    double softness_;
    double temperature_;
    double size_;
    double roundness_;
    double fur_amount_;
};

int main()
{
    std::list<std::string> things = {"same old", "same old"};
    if (fplus::all_the_same(things))
        std::cout << "All things being equal." << std::endl;

    std::vector<cat> cats = {{"Tigger", 5, 5, 5, 5, 5},
        {"Simba", 2, 9, 9, 2, 7},
        {"Muffin", 9, 4, 2, 8, 6},
        {"Garfield", 6, 5, 7, 9, 5}};

    auto cutest_cat = fplus::maximum_on(std::mem_fn(&cat::cuteness), cats);

    std::cout << cutest_cat.name_ << " is happy and sleepy. *purr* *purr* *purr*" << std::endl;
    return 0;
}

When compiling with the aforementioned compiler and standard combination (gcc 11.2 in C++20 mode), I'm receiving this error:

[ 50%] Building CXX object CMakeFiles/main.dir/main.o
In file included from /home/tbreslein/.conan/data/functionalplus/0.2.16/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/fplus/fplus.hpp:11,
                 from /home/tbreslein/coding/playgrounds/fplustest/main.cpp:1:
/home/tbreslein/.conan/data/functionalplus/0.2.16/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/fplus/container_common.hpp:154:42: error: expected unqualified-id before ‘const’
  154 |         array_back_insert_iterator<T, N>(const array_back_insert_iterator<T, N>& other) :
      |                                          ^~~~~
/home/tbreslein/.conan/data/functionalplus/0.2.16/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/fplus/container_common.hpp:154:42: error: expected ‘)’ before ‘const’
  154 |         array_back_insert_iterator<T, N>(const array_back_insert_iterator<T, N>& other) :
      |                                         ~^~~~~
      |                                          )

A little google search reveals that the error is is the <T,N> template parameters attached to the function, and that this is supposedly considered a bug in 'older' compiler versions: https://stackoverflow.com/questions/67698523/error-in-llvm-stlextras-expected-unqualified-id-before-const-with-gcc11

The fix is simple enough, just remove those parameters, so that the constructor in question (container_common.hpp:154 in release 0.2.16) reads as:

        array_back_insert_iterator(const array_back_insert_iterator<T, N>& other) :
            base_type(*other.arr_ptr_), arr_ptr_(other.arr_ptr_), pos_(other.pos_) {}

instead of

        array_back_insert_iterator<T, N>(const array_back_insert_iterator<T, N>& other) :
            base_type(*other.arr_ptr_), arr_ptr_(other.arr_ptr_), pos_(other.pos_) {}

The templated types can still be deduced by the type of input parameter other, so this "should just work".

I "fixed" this in my local clone of the library, and tested it with gcc 11.2 and clang 13.0 in the standards 14, 17, and 20. They all compile and run my little example fine now.

Similarly, I cannot compile your doctests when configuring them with -DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_STANDARD=20 (though, after the fix, I run into the same problem as issue #250 during linking; it does compile the object files successfully so the error I encountered seems entirely gone, I guess?)

So, is there any concern with this little fix? Unfortunately, I currently cannot test this with the other compiler versions you are supporting myself, and won't be able to test it with Visual Studio and XCode at all.

It's a bit of a wall of text, sorry, but I guess it's probably fairly detailed too ^^'

Cheers!

Dobiasd commented 2 years ago

Hi,

and thanks for the very detailed report. :+1:

I can reproduce the problem with the following minimal Dockerfile (g++ 11.2 with C++20):

FROM gcc:11.2

RUN git clone -b 'v0.2.16-p0' --single-branch --depth 1 https://github.com/Dobiasd/FunctionalPlus

RUN echo '#include <fplus/fplus.hpp>' > main.cpp
RUN echo 'int main() {}' >> main.cpp

RUN g++ -I FunctionalPlus/include -std=c++14 main.cpp
RUN g++ -I FunctionalPlus/include -std=c++17 main.cpp
RUN g++ -I FunctionalPlus/include -std=c++20 main.cpp

Output of the last line:

In file included from FunctionalPlus/include/fplus/fplus.hpp:11,
                 from main.cpp:1:
FunctionalPlus/include/fplus/container_common.hpp:154:42: error: expected unqualified-id before 'const'
  154 |         array_back_insert_iterator<T, N>(const array_back_insert_iterator<T, N>& other) :
      |                                          ^~~~~
FunctionalPlus/include/fplus/container_common.hpp:154:42: error: expected ')' before 'const'
  154 |         array_back_insert_iterator<T, N>(const array_back_insert_iterator<T, N>& other) :
      |                                         ~^~~~~
      |                                          )

I'm now testing your suggested fix on a branch. The CI will automatically test it for a whole bunch of different compiler versions. :slightly_smiling_face:

Dobiasd commented 2 years ago

Just released the new version v0.2.17-p0 that includes your fix. :heavy_check_mark:

Thanks again for the good investigation and suggestion. :+1:

tbreslein commented 2 years ago

Cool, you're welcome and thank you for implementing it!