guybrush77 / rapidobj

A fast, header-only, C++17 library for parsing Wavefront .obj files.
MIT License
159 stars 14 forks source link

Move Subdivide method implementations for C++ ≥ 20 compatibility. #23

Closed stripe2933 closed 7 months ago

stripe2933 commented 7 months ago

In Clang 16, example is unable to build when C++20 or C++23, due to type MergeTask depends on CopyElements, CopyIndices, FillElements, FillIds whose structs are defined below the type definition and therefore implicitly instantiated. For this code

#include <iostream>
#include <numeric>

#include "rapidobj.hpp"

int main(int argc, char **argv){
    assert(argc == 2);

    rapidobj::Result result = rapidobj::ParseFile(argv[1]);

    if (result.error) {
        std::cout << result.error.code.message() << '\n';
        return EXIT_FAILURE;
    }

    bool success = rapidobj::Triangulate(result);

    if (!success) {
        std::cout << result.error.code.message() << '\n';
        return EXIT_FAILURE;
    }

    size_t num_triangles = std::reduce(result.shapes.begin(), result.shapes.end(), 0, [](size_t acc, const rapidobj::Shape& shape) {
        return acc + shape.mesh.num_face_vertices.size();
    });

    std::cout << "Shapes:    " << result.shapes.size() << '\n';
    std::cout << "Materials: " << result.materials.size() << '\n';
    std::cout << "Triangles: " << num_triangles << '\n';

    return EXIT_SUCCESS;
}

with build command

clang++ -std=c++17 main.cpp -o main

runs ok, but with version 20 (clang++ -std=c++20 main.cpp -o main)

In file included from main.cpp:1:

[...omitted...]

In file included from /opt/homebrew/opt/llvm/bin/../include/c++/v1/__utility/move.h:15:
/opt/homebrew/opt/llvm/bin/../include/c++/v1/__type_traits/is_copy_constructible.h:27:11: error: incomplete type 'rapidobj::detail::CopyIndices' used in type trait expression
          __is_constructible(_Tp, __add_lvalue_reference_t<typename add_const<_Tp>::type>)> {};
          ^
/opt/homebrew/opt/llvm/bin/../include/c++/v1/__type_traits/is_copy_constructible.h:31:49: note: in instantiation of template class 'std::is_copy_constructible<rapidobj::detail::CopyIndices>' requested here
inline constexpr bool is_copy_constructible_v = is_copy_constructible<_Tp>::value;
                                                ^
/opt/homebrew/opt/llvm/bin/../include/c++/v1/variant:1296:17: note: in instantiation of variable template specialization 'std::is_copy_constructible_v<rapidobj::detail::CopyIndices>' requested here
          __all<is_copy_constructible_v<_Types>...>::value,
                ^
/opt/homebrew/opt/llvm/bin/../include/c++/v1/vector:549:52: note: in instantiation of template class 'std::variant<rapidobj::detail::CopyElements<unsigned char>, rapidobj::detail::CopyElements<int>, rapidobj::detail::CopyElements<float>, rapidobj::detail::CopyIndices, rapidobj::detail::FillElements<float>, rapidobj::detail::FillIds<int>, rapidobj::detail::FillIds<unsigned int>>' requested here
        {return static_cast<size_type>(__end_cap() - this->__begin_);}
                                                   ^
/opt/homebrew/opt/llvm/bin/../include/c++/v1/vector:763:56: note: in instantiation of member function 'std::vector<std::variant<rapidobj::detail::CopyElements<unsigned char>, rapidobj::detail::CopyElements<int>, rapidobj::detail::CopyElements<float>, rapidobj::detail::CopyIndices, rapidobj::detail::FillElements<float>, rapidobj::detail::FillIds<int>, rapidobj::detail::FillIds<unsigned int>>>::capacity' requested here
      __annotate_contiguous_container(data(), data() + capacity(),
                                                       ^
/opt/homebrew/opt/llvm/bin/../include/c++/v1/vector:442:18: note: in instantiation of member function 'std::vector<std::variant<rapidobj::detail::CopyElements<unsigned char>, rapidobj::detail::CopyElements<int>, rapidobj::detail::CopyElements<float>, rapidobj::detail::CopyIndices, rapidobj::detail::FillElements<float>, rapidobj::detail::FillIds<int>, rapidobj::detail::FillIds<unsigned int>>>::__annotate_delete' requested here
          __vec_.__annotate_delete();
                 ^
/opt/homebrew/opt/llvm/bin/../include/c++/v1/vector:456:67: note: in instantiation of member function 'std::vector<std::variant<rapidobj::detail::CopyElements<unsigned char>, rapidobj::detail::CopyElements<int>, rapidobj::detail::CopyElements<float>, rapidobj::detail::CopyIndices, rapidobj::detail::FillElements<float>, rapidobj::detail::FillIds<int>, rapidobj::detail::FillIds<unsigned int>>>::__destroy_vector::operator()' requested here
  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI ~vector() { __destroy_vector(*this)(); }
                                                                  ^
./rapidobj.hpp:4616:22: note: in instantiation of member function 'std::vector<std::variant<rapidobj::detail::CopyElements<unsigned char>, rapidobj::detail::CopyElements<int>, rapidobj::detail::CopyElements<float>, rapidobj::detail::CopyIndices, rapidobj::detail::FillElements<float>, rapidobj::detail::FillIds<int>, rapidobj::detail::FillIds<unsigned int>>>::~vector' requested here
        auto tasks = MergeTasks();
                     ^
./rapidobj.hpp:4582:8: note: forward declaration of 'rapidobj::detail::CopyIndices'

[...omitted...]

fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.

I moved the implementation of Subdivide methods to below the struct definitions, and now it can be built for c++ ≥ 17.

guybrush77 commented 7 months ago

Thanks for the PR!