search

boostorg / spirit

Boost.org spirit module
http://boost.org/libs/spirit
394 stars 162 forks source link

MSVC (VS 2023) fails to compile example/x3/annotation.cpp with /std:c++20 or /std:c++latest option #782

Closed voivoid closed 9 months ago

voivoid commented 10 months ago

MSVC fails to compile annotation example if /std:c++20 or /std:c++latest is set. Everything is fine with /std:c++17.

See compiler explorer to get the bug reproduction

Unfortunately the error message ( can be seen in the provided link ) is pretty cryptic so I'm not sure whether it's a MSVC or boost::spirit problem.

StephanTLavavej commented 9 months ago

This was also reported to MSVC as DevCom-10565526 "boost::spirit fails to compile with /std:c++20 with MSVC 19.34+". It happens with /std:c++20 and VS 2022 17.4 because we implemented P2520R0 "move_iterator<T*> should be a random access iterator" (which is a C++23 feature) "downlevel" in C++20 mode (treating it as a defect report, in alignment with libstdc++ and libc++). Both MSVC's compiler and clang-cl repro this with MSVC's STL, and clang-cl emits a more understandable error.

Click to expand clang-cl compiler error (VS 2022 17.10 Preview 1 x64, Clang 17.0.3, Boost 1.84.0): ``` C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++20 /MTd /Od /c /I boost_1_84_0\boost_1_84_0 -Xclang -ftemplate-backtrace-limit=0 meow.cpp In file included from meow.cpp:4: In file included from boost_1_84_0\boost_1_84_0\boost/spirit/home/x3.hpp:67: In file included from boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator.hpp:10: boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator/sequence.hpp(63,24): error: no viable overloaded operator[] for type 'const expect_gen' 63 | return left >> expect[right]; | ^~~~~~ ~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\concepts(224,11): note: in instantiation of function template specialization 'boost::spirit::x3::operator>>>, std::_Vector_iterator>>>' requested here 224 | { __t > __u } -> _Boolean_testable; | ^ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\concepts(224,7): note: in instantiation of requirement here 224 | { __t > __u } -> _Boolean_testable; | ^~~~~~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\concepts(222,25): note: while substituting template arguments into constraint expression here 222 | concept _Half_ordered = requires(const remove_reference_t<_Ty1>& __t, const remove_reference_t<_Ty2>& __u) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 223 | { __t < __u } -> _Boolean_testable; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 224 | { __t > __u } -> _Boolean_testable; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 225 | { __t <= __u } -> _Boolean_testable; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 226 | { __t >= __u } -> _Boolean_testable; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 227 | }; | ~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\concepts(233,55): note: while checking the satisfaction of concept '_Half_ordered>>, std::_Vector_iterator>>>' requested here 233 | concept totally_ordered = equality_comparable<_Ty> && _Half_ordered<_Ty, _Ty>; | ^~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\concepts(233,55): note: while substituting template arguments into constraint expression here 233 | concept totally_ordered = equality_comparable<_Ty> && _Half_ordered<_Ty, _Ty>; | ^~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\xutility(804,72): note: while checking the satisfaction of concept 'totally_ordered>>>' requested here 804 | && derived_from<_Iter_concept<_It>, random_access_iterator_tag> && totally_ordered<_It> | ^~~~~~~~~~~~~~~~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\xutility(804,72): note: while substituting template arguments into constraint expression here 804 | && derived_from<_Iter_concept<_It>, random_access_iterator_tag> && totally_ordered<_It> | ^~~~~~~~~~~~~~~~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\xutility(3980,23): note: while checking the satisfaction of concept 'random_access_iterator>>>' requested here 3980 | if constexpr (random_access_iterator<_Iter>) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\xutility(3992,39): note: in instantiation of member function 'std::move_iterator>>>::_Get_iter_concept' requested here 3992 | using iterator_concept = decltype(_Get_iter_concept()); | ^ C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.40.33521\include\xutility(4308,46): note: in instantiation of template class 'std::move_iterator>>>' requested here 4308 | _NODISCARD _CONSTEXPR17 move_iterator<_Iter> make_move_iterator(_Iter _It) noexcept( | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator/detail/sequence.hpp(383,39): note: in instantiation of function template specialization 'std::make_move_iterator>>>' requested here 383 | traits::append(attr, std::make_move_iterator(traits::begin(attr_)), | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator/detail/sequence.hpp(412,20): note: in instantiation of function template specialization 'boost::spirit::x3::detail::parse_into_container_impl, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, boost::spirit::x3::unused_type, const boost::spirit::x3::unused_type>::call>>, std::vector>' requested here 412 | return call(parser, first, last, context, rcontext, attr | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/core/detail/parse_into_container.hpp(303,70): note: in instantiation of function template specialization 'boost::spirit::x3::detail::parse_into_container_impl, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, boost::spirit::x3::unused_type, const boost::spirit::x3::unused_type>::call>>, std::vector>' requested here 303 | return parse_into_container_impl::call( | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator/detail/sequence.hpp(282,16): note: in instantiation of function template specialization 'boost::spirit::x3::detail::parse_into_container, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, std::_String_const_iterator>>, boost::spirit::x3::unused_type, const boost::spirit::x3::unused_type, std::vector>' requested here 282 | return parse_into_container(parser, first, last, context, rcontext, attr); | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator/detail/sequence.hpp(293,13): note: in instantiation of function template specialization 'boost::spirit::x3::detail::parse_sequence_container, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, std::_String_const_iterator>>, boost::spirit::x3::unused_type, const boost::spirit::x3::unused_type, std::vector>' requested here 293 | if (parse_sequence_container(parser.left, first, last, context, rcontext, attr) | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/operator/sequence.hpp(46,28): note: in instantiation of function template specialization 'boost::spirit::x3::detail::parse_sequence, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, boost::spirit::x3::literal_string>, std::_String_const_iterator>>, boost::spirit::x3::unused_type, const boost::spirit::x3::unused_type, std::vector>' requested here 46 | return detail::parse_sequence(*this, first, last, context, rcontext, attr | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/core/parse.hpp(36,29): note: in instantiation of function template specialization 'boost::spirit::x3::sequence, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, boost::spirit::x3::literal_string>::parse>>, boost::spirit::x3::unused_type, const boost::spirit::x3::unused_type, std::vector>' requested here 36 | return as_parser(p).parse(first, last, unused, unused, attr); | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/core/parse.hpp(61,16): note: in instantiation of function template specialization 'boost::spirit::x3::parse_main>>, boost::spirit::x3::sequence, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, boost::spirit::x3::literal_string>, std::vector>' requested here 61 | return parse_main(first, last, p, attr); | ^ meow.cpp(35,9): note: in instantiation of function template specialization 'boost::spirit::x3::parse>>, boost::spirit::x3::sequence, boost::spirit::x3::list, boost::spirit::x3::literal_string>>, boost::spirit::x3::literal_string>, std::vector>' requested here 35 | x3::parse(parsed.cbegin(), parsed.cend(), itemsRule, attribute); | ^ boost_1_84_0\boost_1_84_0\boost/spirit/home/x3/directive/expect.hpp(70,9): note: candidate template ignored: substitution failure [with Subject = std::_Vector_iterator>>]: no type named 'value_type' in 'boost::spirit::x3::extension::as_parser>>>' 69 | constexpr expect_directive::value_type> | ~~~~~~~~~~ 70 | operator[](Subject const& subject) const | ^ 1 error generated. ```

What's happening is that boost_1_84_0\boost/spirit/home/x3/operator/detail/sequence.hpp(383,39) is calling std::make_move_iterator() on a std::vector<ast::Item>::iterator. Due to P2520R0, std::move_iterator has to ask whether the wrapped iterator models std::random_access_iterator. Obviously (to humans) this is true for a std::vector::iterator, but the library doesn't short-circuit that check, it has to actually evaluate the concept. This has to ask whether the iterator is std::totally_ordered, and that ends up asking whether the std::vector<ast::Item>::iterators can be compared with >.

:zap: Argument-dependent lookup strikes! :zap:

This ends up considering the following unconstrained operator> overload in boost::spirit::x3, instantiating it for std::vector<ast::Item>::iterator:

https://github.com/boostorg/spirit/blob/f39edf226afc3d7b7d5c84a6c63fe3eaccc1e21b/include/boost/spirit/home/x3/operator/sequence.hpp#L60-L64

The std::totally_ordered concept (actually our implementation detail std::_Half_ordered, but that doesn't matter) just wants to know whether vector_iterator > vector_iterator is well-formed and boolean-testable, but while performing overload resolution, it has to consider Boost.Spirit's unconstrained operator>, and the return type is auto so it has to instantiate the operator's function body. This causes a hard (non-SFINAE-able) compiler error because whatever left >> expect[right] does, it is not prepared for this to be used with std::vector<ast::Item>::iterator.

Therefore, this is a bug in Boost.Spirit, not MSVC. (I can't explain why it doesn't repro with libstdc++ and libc++, even with their debug modes - the implementation details of std::vector may be relevant.) I believe the fix would involve constraining this boost::spirit::x3::operator> overload, but I'm not 100% sure. Hope this helps!

AlexGuteniev commented 9 months ago

I believe the fix would involve constraining this boost::spirit::x3::operator> overload

Implementing own move_iterator might be a way forward to avoid using C++20 features (and relying on conditional compilation to support C++17)

Kojoley commented 9 months ago

All operators are constrained by SFINAE on return type except operator> , that's inconsistent. But I also thinking about moving types users can inherit from to a separate namespace because SFINAEd-out operators unnecessarily pollutes compiler error messages (which are on their own are hard for users to understand).