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

Issues with constness in std::make_pair #61

Closed
mskeffingtonroku opened this issue May 19, 2022 · 4 comments
Closed

Issues with constness in std::make_pair #61

mskeffingtonroku opened this issue May 19, 2022 · 4 comments

Comments

@mskeffingtonroku
Copy link

mskeffingtonroku commented May 19, 2022

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
Copy link
Author

This warning is generated by gcc12

@breese
Copy link
Owner

breese commented May 20, 2022

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
Copy link
Author

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

@breese
Copy link
Owner

breese commented May 23, 2022

Fixed in 9b764e9.

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

@breese breese closed this as completed May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants