Ryan-rsm-McKenzie / bsa

C++ library for working with the Bethesda archive file format
MIT License
41 stars 6 forks source link

Fix compilation on Clang #4

Closed Guekka closed 2 years ago

Ryan-rsm-McKenzie commented 2 years ago

clang doesn't support enough of c++20 to successfully compile bsa

Guekka commented 2 years ago

With this fix, clang-cl 13 can compile it. Haven't tried on Linux. But it allows me to run clang tidy, so it's already better

Ryan-rsm-McKenzie commented 2 years ago

This most definitely does not compile with clang-13

Guekka commented 2 years ago

Well, it does work with clang-cl on my computer. I don't know more. I can give you my detailed setup if you want

Ryan-rsm-McKenzie commented 2 years ago

Try building bsa and its dependencies with clang proper on wsl

Guekka commented 2 years ago

Alright, I'm gonna try. But please, I will appreciate if you merge this no matter the outcome. It's not like it's a big change 

Guekka commented 2 years ago

Alright, I can confirm it doesn't compile on Linux. So I don't get how it could work on my Windows machine. Anyway, the only part that isn't fixable is this one:

    [[nodiscard]] auto derive() noexcept
            -> derived_type&
        {
            static_assert(
                concepts::input_stream<derived_type>,
                "derived type does not meet the minimum requirements for being an input stream");
            return static_cast<derived_type &>(*this);
        }

Apparently, clang support for concepts is not yet complete. But I suppose you already knew that

Ryan-rsm-McKenzie commented 2 years ago

This is not the only problem, clang still doesn't support lambda captures for structured bindings, which are used extensively throughout the codebase https://godbolt.org/z/7bo8qqc9d

Guekka commented 2 years ago

This one is easily fixable https://godbolt.org/z/xaGK6fdrh Not very pretty, but I only saw this pattern occur thrice in the codebase

Ryan-rsm-McKenzie commented 2 years ago

This is not the kind of workaround I would find acceptable

Guekka commented 2 years ago

It's your choice. I'll close this PR then. Thanks

Ryan-rsm-McKenzie commented 2 years ago

There is a partial fix merged https://github.com/Ryan-rsm-McKenzie/bsa/commit/d88fce17274bdfe701e9c193fa26f3b789c03810