Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GltfImporter: fix libc++15 build #131

Closed
wants to merge 2 commits into from

Conversation

sthalik
Copy link
Contributor

@sthalik sthalik commented Oct 19, 2022

No description provided.

@mosra
Copy link
Owner

mosra commented Oct 19, 2022

Strange, my MSVC CIs passed even without this include, so I wonder what am I missing in my test setup...

  • Anything specific with your setup?
  • MSVC version?
  • Compiling with the default (C++14) or a higher standard?
  • What's the error message it gives otherwise?

@sthalik
Copy link
Contributor Author

sthalik commented Oct 19, 2022

Oops. It's not MSVC but native clang 15.0.2 (C++20; mingw-w64 CRT; libc++; no MSVC STL; no GNU).

The error is:

c:/msys64/clang64/include/c++/v1/__algorithm/iterator_operations.h:100:92: error: no type named 'reference' in 'std::iterator_traits<Corrade::Containers::StridedIterator<1, Corrade::Containers::Pair<Corrade::Containers::BasicStringView<const char>, unsigned int>>>'
    static_assert(is_same<__deref_t<_Iter>, typename iterator_traits<__uncvref_t<_Iter> >::reference>::value,
                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
c:/msys64/clang64/include/c++/v1/__algorithm/iterator_operations.h:115:5: note: in instantiation of function template specialization 'std::_IterOps<std::_ClassicAlgPolicy>::__validate_iter_reference<Corrade::Containers::StridedIterator<1, Corrade::Containers::Pair<Corrade::Containers::BasicStringView<const char>, unsigned int>> &>' requested here
    __validate_iter_reference<_Iter>();
    ^
c:/msys64/clang64/include/c++/v1/__algorithm/unique.h:38:44: note: in instantiation of function template specialization 'std::_IterOps<std::_ClassicAlgPolicy>::__iter_move<Corrade::Containers::StridedIterator<1, Corrade::Containers::Pair<Corrade::Containers::BasicStringView<const char>, unsigned int>> &>' requested here
        *++__first = _IterOps<_AlgPolicy>::__iter_move(__i);

@mosra
Copy link
Owner

mosra commented Oct 19, 2022

Ah, okay. There's already one such conditional include a bit below at

#ifdef CORRADE_MSVC2015_COMPATIBILITY
/* Otherwise std::unique() fails to compile on MSVC 2015. Other compilers are
fine without. */
#include <Corrade/Containers/StridedArrayViewStl.h>
#endif

If you change it to #if defined(CORRADE_MSVC2015_COMPATIBILITY) || (defined(CORRADE_TARGET_MINGW) && defined(CORRADE_TARGET_LIBCXX)) instead of adding the same include unconditionally, does that work for you as well?

I still need to investigate locally if it's something that affects libc++ elsewhere, because it feels strange it would have this problem only on one platform.

@sthalik
Copy link
Contributor Author

sthalik commented Oct 19, 2022

I think it's going to happen on all platforms clang 15 + libc++ is invoked on. At least there's nothing mingw-specific.

@sthalik sthalik force-pushed the pr/fix-gltf-importer-stl-msvc branch from 7238851 to 43f7ddf Compare October 19, 2022 09:21
Complains about `std::unique`.
@sthalik sthalik force-pushed the pr/fix-gltf-importer-stl-msvc branch from 43f7ddf to 5d92a23 Compare October 19, 2022 09:23
@mosra mosra added this to the 2022.0a milestone Oct 19, 2022
@mosra mosra changed the title GltfImporter: fix msvc build GltfImporter: fix libc++15 build Oct 19, 2022
Use @mosra's #ifdef check,

Co-authored-by: Vladimír Vondruš <mosra@centrum.cz>
@codecov
Copy link

codecov bot commented Oct 19, 2022

Codecov Report

Base: 96.81% // Head: 96.81% // No change to project coverage 👍

Coverage data is based on head (e003f7c) compared to base (6da185c).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #131   +/-   ##
=======================================
  Coverage   96.81%   96.81%           
=======================================
  Files         130      130           
  Lines       13902    13902           
=======================================
  Hits        13459    13459           
  Misses        443      443           
Impacted Files Coverage Δ
src/MagnumPlugins/GltfImporter/GltfImporter.cpp 99.39% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mosra
Copy link
Owner

mosra commented Oct 19, 2022

Merged as c8bd027, thanks!

@mosra mosra closed this Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants