Wdeprecated-copy on system type

Wdeprecated-copy on system type

[Csaba Raduly]

#include <boost/range/sub_range.hpp>
#include <string>
#include <vector>

using srange = boost::sub_range<std::string const>;

void func(srange) // by value
{}

int main()
{
    std::string  kitty{"meow"};
    srange hello{kitty};
    srange const& helref = hello;

    func(hello);
    func(helref); // line 17

    return 0;
}

$ g++ -v
gcc version 9.0.0 20180626 (experimental) (GCC)

$ g++ -Wall -pedantic -Wextra -Werror -save-temps subra.cpp
subra.cpp: In function ‘int main()’:
subra.cpp:17:13: error: implicitly-declared ‘constexpr
boost::sub_range<const std::__cxx11::basic_string<char>
>::sub_range(const boost::sub_range<const
std::__cxx11::basic_string<char> >&)’ is deprecated
[-Werror=deprecated-copy]
  func(helref);
             ^
In file included from subra.cpp:1:
/usr/include/boost/range/sub_range.hpp:259:20: note: because
‘boost::sub_range<const std::__cxx11::basic_string<char> >’ has
user-provided ‘boost::sub_range<ForwardRange>&
boost::sub_range<ForwardRange>::operator=(const
boost::sub_range<ForwardRange>&) [with ForwardRange = const
std::__cxx11::basic_string<char>]’
         sub_range& operator=( const sub_range& r )
                    ^~~~~~~~
subra.cpp:7:6: note:   initializing argument 1 of ‘void func(srange)’
 void func(srange) // by value
      ^~~~


I have two questions:

1. Why is a warning emitted only for the const srange?

2. Shouldn't boost be exempt from this warning, given that it's
included from a system directory (/usr/include) ?

Shouldn't the "implicitly declared copy constructor" error be located
at the class definition (boost header), rather than where it's being
called (the call which does pass-by-value) ?

Boost is version 1.62.0.1 packaged by Ubuntu 17.10, so I can't change that.

Csaba
Please CC me, I'm not subscribed to the list.
-- 
You can get very substantial performance improvements
by not doing the right thing. - Scott Meyers, An Effective C++11/14 Sampler
So if you're looking for a completely portable, 100% standards-conformat way
to get the wrong information: this is what you want. - Scott Meyers (C++TDaWYK)

Re: Wdeprecated-copy on system type

[Jonathan Wakely]

Your mail is off-topic on this list, it would be appropriate on the
gcc-help list, or in Bugzilla if you want to report a bug.


On Wed, 27 Jun 2018 at 11:27, Csaba Raduly wrote:
>
> #include <boost/range/sub_range.hpp>
> #include <string>
> #include <vector>
>
> using srange = boost::sub_range<std::string const>;
>
> void func(srange) // by value
> {}
>
> int main()
> {
>     std::string  kitty{"meow"};
>     srange hello{kitty};
>     srange const& helref = hello;
>
>     func(hello);
>     func(helref); // line 17
>
>     return 0;
> }
>
> $ g++ -v
> gcc version 9.0.0 20180626 (experimental) (GCC)
>
> $ g++ -Wall -pedantic -Wextra -Werror -save-temps subra.cpp
> subra.cpp: In function ‘int main()’:
> subra.cpp:17:13: error: implicitly-declared ‘constexpr
> boost::sub_range<const std::__cxx11::basic_string<char>
> >::sub_range(const boost::sub_range<const
> std::__cxx11::basic_string<char> >&)’ is deprecated
> [-Werror=deprecated-copy]
>   func(helref);
>              ^
> In file included from subra.cpp:1:
> /usr/include/boost/range/sub_range.hpp:259:20: note: because
> ‘boost::sub_range<const std::__cxx11::basic_string<char> >’ has
> user-provided ‘boost::sub_range<ForwardRange>&
> boost::sub_range<ForwardRange>::operator=(const
> boost::sub_range<ForwardRange>&) [with ForwardRange = const
> std::__cxx11::basic_string<char>]’
>          sub_range& operator=( const sub_range& r )
>                     ^~~~~~~~
> subra.cpp:7:6: note:   initializing argument 1 of ‘void func(srange)’
>  void func(srange) // by value
>       ^~~~
>
>
> I have two questions:
>
> 1. Why is a warning emitted only for the const srange?

Because line 16 doesn't use the sub_range copy constructor, it calls
this instead:

        template< class ForwardRange2 >
        sub_range(
            ForwardRange2& r,
            BOOST_DEDUCED_TYPENAME ::boost::enable_if<
                is_compatible_range<ForwardRange2>
            >::type* = 0
        )
        : base(impl::adl_begin(r), impl::adl_end(r))
        {
        }

This might be a Boost bug, albeit a harmless one. IMHO the template
constructor should be constrained so it isn't used for copying
non-const lvalues.


> 2. Shouldn't boost be exempt from this warning, given that it's
> included from a system directory (/usr/include) ?

Yes, it probably should, but see below.

> Shouldn't the "implicitly declared copy constructor" error be located
> at the class definition (boost header), rather than where it's being
> called (the call which does pass-by-value) ?

The warning is only issued at the point where the implicit definition
is needed, to avoid issuing warnings when the object is never copied.
It's needed at the call site in your code, not in the system header
(well, it might also be needed in the boost headers, but those
warnings are probably suppressed).

But I agree that if the class is defined in a system header you can't
change the code, so the warning is not very helpful. Please report a
bug, https://gcc.gnu.org/bugs/

Re: Wdeprecated-copy on system type

[Csaba Raduly]

On Wed, Jun 27, 2018 at 1:28 PM, Jonathan Wakely wrote:
> Your mail is off-topic on this list, it would be appropriate on the
> gcc-help list, or in Bugzilla if you want to report a bug.
>
>
> On Wed, 27 Jun 2018 at 11:27, Csaba Raduly wrote:
(snip)
>> 1. Why is a warning emitted only for the const srange?
>
> Because line 16 doesn't use the sub_range copy constructor, it calls
> this instead:
>
>         template< class ForwardRange2 >
>         sub_range(
>             ForwardRange2& r,
>             BOOST_DEDUCED_TYPENAME ::boost::enable_if<
>                 is_compatible_range<ForwardRange2>
>             >::type* = 0
>         )
>         : base(impl::adl_begin(r), impl::adl_end(r))
>         {
>         }
>
> This might be a Boost bug, albeit a harmless one. IMHO the template
> constructor should be constrained so it isn't used for copying
> non-const lvalues.

So the fact that there's no warning for the non-const version is in
fact a bug in Boost, right?

(It's still like that in latest, not yet released Boost 1.68).

This sounds like a "greedy template" you spoke about at ACCUcon 2018.

>
>
>> 2. Shouldn't boost be exempt from this warning, given that it's
>> included from a system directory (/usr/include) ?
>
> Yes, it probably should, but see below.
>
>> Shouldn't the "implicitly declared copy constructor" error be located
>> at the class definition (boost header), rather than where it's being
>> called (the call which does pass-by-value) ?
>
> The warning is only issued at the point where the implicit definition
> is needed, to avoid issuing warnings when the object is never copied.
> It's needed at the call site in your code, not in the system header
> (well, it might also be needed in the boost headers, but those
> warnings are probably suppressed).

Of course the warning should only be emitted when such a
constructor/assignment is used.

Still, in my naive opinion, such a constructor is declared
(implicitly) in the class (sub_range).
The message at the point of use should be something similar to the
"instantiated from" message when there's an error inside a template.

Something like:

/usr/include/boost/range/sub_range.hpp:???: implicitly-declared ‘constexpr
boost::sub_range<const
std::__cxx11::basic_string<char>>::sub_range(const
boost::sub_range<const std::__cxx11::basic_string<char> >&)’ is
deprecated
[-Werror=deprecated-copy]

/usr/include/boost/range/sub_range.hpp:259:20: note: because REASONS

subra.cpp:7:6: note:   used when initializing argument 1 of ‘void func(srange)’

>
> But I agree that if the class is defined in a system header you can't
> change the code, so the warning is not very helpful. Please report a
> bug, https://gcc.gnu.org/bugs/


Csaba
-- 
You can get very substantial performance improvements
by not doing the right thing. - Scott Meyers, An Effective C++11/14 Sampler
So if you're looking for a completely portable, 100% standards-conformat way
to get the wrong information: this is what you want. - Scott Meyers (C++TDaWYK)

Re: Wdeprecated-copy on system type

[Jonathan Wakely]

On Thu, 28 Jun 2018 at 12:32, Csaba Raduly wrote:
>
> On Wed, Jun 27, 2018 at 1:28 PM, Jonathan Wakely wrote:
> > Your mail is off-topic on this list, it would be appropriate on the
> > gcc-help list, or in Bugzilla if you want to report a bug.
> >
> >
> > On Wed, 27 Jun 2018 at 11:27, Csaba Raduly wrote:
> (snip)
> >> 1. Why is a warning emitted only for the const srange?
> >
> > Because line 16 doesn't use the sub_range copy constructor, it calls
> > this instead:
> >
> >         template< class ForwardRange2 >
> >         sub_range(
> >             ForwardRange2& r,
> >             BOOST_DEDUCED_TYPENAME ::boost::enable_if<
> >                 is_compatible_range<ForwardRange2>
> >             >::type* = 0
> >         )
> >         : base(impl::adl_begin(r), impl::adl_end(r))
> >         {
> >         }
> >
> > This might be a Boost bug, albeit a harmless one. IMHO the template
> > constructor should be constrained so it isn't used for copying
> > non-const lvalues.
>
> So the fact that there's no warning for the non-const version is in
> fact a bug in Boost, right?

I think that's for the Boost maintainers to decide. The constructor
template still gives the right behaviour, even if it's probably not
what was intended (a sub_range<T>& is a compatible range of a
sub_range<T>, and so can be "converted" to it by that constructor). It
probably makes more sense to constrain that constructor so it's not
used for "conversions" from sub_range<T>& to sub_range<T>, and the
copy constructor gets used instead.


> (It's still like that in latest, not yet released Boost 1.68).
>
> This sounds like a "greedy template" you spoke about at ACCUcon 2018.

Yes, exactly.

There are other  cases in Boost which cause bigger problems e.g.
https://wandbox.org/permlink/9Py2Nu8ni6qnSU7O which crashes because
trying to copy-construct a non-const lvalue will keep trying to wrap
itself recursively until it overflows the stack.


> >> 2. Shouldn't boost be exempt from this warning, given that it's
> >> included from a system directory (/usr/include) ?
> >
> > Yes, it probably should, but see below.
> >
> >> Shouldn't the "implicitly declared copy constructor" error be located
> >> at the class definition (boost header), rather than where it's being
> >> called (the call which does pass-by-value) ?
> >
> > The warning is only issued at the point where the implicit definition
> > is needed, to avoid issuing warnings when the object is never copied.
> > It's needed at the call site in your code, not in the system header
> > (well, it might also be needed in the boost headers, but those
> > warnings are probably suppressed).
>
> Of course the warning should only be emitted when such a
> constructor/assignment is used.
>
> Still, in my naive opinion, such a constructor is declared
> (implicitly) in the class (sub_range).
> The message at the point of use should be something similar to the
> "instantiated from" message when there's an error inside a template.
>
> Something like:
>
> /usr/include/boost/range/sub_range.hpp:???: implicitly-declared ‘constexpr
> boost::sub_range<const
> std::__cxx11::basic_string<char>>::sub_range(const
> boost::sub_range<const std::__cxx11::basic_string<char> >&)’ is
> deprecated
> [-Werror=deprecated-copy]
>
> /usr/include/boost/range/sub_range.hpp:259:20: note: because REASONS
>
> subra.cpp:7:6: note:   used when initializing argument 1 of ‘void func(srange)’

Yes, please report it to our bugzilla.

> >
> > But I agree that if the class is defined in a system header you can't
> > change the code, so the warning is not very helpful. Please report a
> > bug, https://gcc.gnu.org/bugs/

Re: Wdeprecated-copy on system type

[Jonathan Wakely]

On Thu, 28 Jun 2018 at 12:49, Jonathan Wakely wrote:
> Yes, please report it to our bugzilla.

Ah, I've just seen https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86342 - thanks!