breese / trial.protocol

Network wire protocols
11 stars 4 forks source link

Issues with constness in std::make_pair #61

Closed mskeffingtonroku closed 2 years ago

mskeffingtonroku commented 2 years ago
include/c++/12.1.0/bits/stl_pair.h: In instantiation of 'struct std::pair<const trial::dynamic::basic_variable<std::allocator<char> >, trial::dynamic::basic_variable<std::allocator<char> > >':                                                                                                                                
include/c++/12.1.0/type_traits:1467:45:   required by substitution of 'template<class _From1, class _To1, class> static std::true_type std::__is_convertible_helper<const trial::dynamic::basic_variable<std::allocator<char> >&, trial::dynamic::basic_variable<std::allocator<char> >, false>::__test(int) [with _From1 = const trial::dynamic::basic_variable<std::allocator<char> >&; _To1 = trial::dynamic::basic_variable<std::allocator<char> >; <template-parameter-1-3> = <missing>]'                                      
include/c++/12.1.0/type_traits:1476:42:   required from 'struct std::__is_convertible_helper<const trial::dynamic::basic_variable<std::allocator<char> >&, trial::dynamic::basic_variable<std::allocator<char> >, false>'                                                                                                       
include/c++/12.1.0/type_traits:1482:12:   required from 'struct std::is_convertible<const trial::dynamic::basic_variable<std::allocator<char> >&, trial::dynamic::basic_variable<std::allocator<char> > >'                                                                                                                      
include/c++/12.1.0/type_traits:3284:72:   required from 'constexpr const bool std::is_convertible_v<const trial::dynamic::basic_variable<std::allocator<char> >&, trial::dynamic::basic_variable<std::allocator<char> > >'                                                                                                      
include/c++/12.1.0/bits/stl_pair.h:259:18:   required from 'static constexpr bool std::pair<_T1, _T2>::_S_convertible() [with _U1 = const trial::dynamic::basic_variable<std::allocator<char> >&; _U2 = const trial::dynamic::basic_variable<std::allocator<char> >&; _T1 = trial::dynamic::basic_variable<std::allocator<char> >; _T2 = trial::dynamic::basic_variable<std::allocator<char> >]'                                                                                                                                    
include/c++/12.1.0/bits/stl_pair.h:268:65:   required from 'struct std::pair<trial::dynamic::basic_variable<std::allocator<char> >, trial::dynamic::basic_variable<std::allocator<char> > >' 

include/trial/protocol/bintoken/serialization/dynamic/variable.hpp:121:44:   required from here

include/c++/12.1.0/bits/stl_pair.h:268:65:   in 'constexpr' expansion of 'std::pair<const trial::dynamic::basic_variable<std::allocator<char> >, trial::dynamic::basic_variable<std::allocator<char> > >::_S_convertible<const trial::dynamic::basic_variable<std::allocator<char> >&, const trial::dynamic::basic_variable<std::allocator<char> >&>()'                                                                                                                                                                             
include/c++/12.1.0/bits/stl_pair.h:260:20: error: the value of 'std::is_convertible_v<const trial::dynamic::basic_variable<std::allocator<char> >&, trial::dynamic::basic_variable<std::allocator<char> > >' is not usable in a constant expression                                                                             
  260 |             return is_convertible_v<_U2, _T2>;                                                                                                                                              
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                               
In file included from include/c++/12.1.0/bits/char_traits.h:42,                                                             
                 from include/c++/12.1.0/string:40:                                                                         
include/c++/12.1.0/type_traits:3284:25: note: 'std::is_convertible_v<const trial::dynamic::basic_variable<std::allocator<char> >&, trial::dynamic::basic_variable<std::allocator<char> > >' used in its own initializer                                                                                                         
 3284 |   inline constexpr bool is_convertible_v = is_convertible<_From, _To>::value;

this patch solves it

+++ b/bintoken/serialization/dynamic/variable.hpp   2022-05-11 15:15:31.021192091 -0700
@@ -118,7 +118,9 @@ struct save_overloader< protocol::bintok
             ar.template save<bintoken::token::null>();
             for (auto it = data.begin(); it != data.end(); ++it)
             {
-                auto value = std::make_pair(it.key(), it.value());
+                auto key = it.key();
+                auto val = it.value();
+                auto value = std::make_pair(key, val);
                 ar.save_override(value.first, protocol_version);
                 ar.save_override(value.second, protocol_version);
             }
mskeffingtonroku commented 2 years ago

This warning is generated by gcc12

breese commented 2 years ago

Do you have a minimal example that triggers this error? I would like to add a regression test, but have not been able to reproduce it yet.

Does it also work is you move the two parameters? E.g. auto value = std::make_pair(std::move(key), std::move(value)). If so, we can avoid some unnecessary copying.

mskeffingtonroku commented 2 years ago

This was generated building Strom with gcc12. Using move works. Good alternative!

breese commented 2 years ago

Fixed in 9b764e9ce4548ea514a1cfc862208ba4195d85f7.

You may need to talk to Jacob Pedersen to upgrade Strom.