Re: [analyzer PATCH] Restore bootstrap with g++ 4.8.

[Jonathan Wakely <jwakely@redhat.com>]

On 14/06/24 11:42 +0100, Jonathan Wakely wrote:
>On 14/06/24 11:37 +0100, Jonathan Wakely wrote:
>>On 14/06/24 11:26 +0100, Jonathan Wakely wrote:
>>>On 14/06/24 10:36 +0100, Jonathan Wakely wrote:
>>>>On 07/06/24 19:40 +0100, Roger Sayle wrote:
>>>>>
>>>>>This patch restores bootstrap when using g++ 4.8 as a host compiler.
>>>>>Returning a std::unique_ptr requires a std::move on C++ compilers
>>>>>(pre-C++17) that don't guarantee copy elision/return value optimization.
>>>>
>>>>It doesn't though.  The C++17 guaranteed copy elision rules are not
>>>>relevant here.  This is about lookup for the constructor used in the
>>>>return statement, and whether that lookup considers the variable to be
>>>>an lvalue or an rvalue.  C++11 already says this is valid:
>>>>
>>>>i#include <memory>
>>>>
>>>>std::unique_ptr<int> f()
>>>>{
>>>> std::unique_ptr<int> m;
>>>> return m;
>>>>}
>>>>
>>>>See C++11 12.8 [class.copy] p31:
>>>>
>>>>This elision of copy/move operations, called copy elision, is
>>>>permitted in the following circumstances (which may be combined to
>>>>eliminate multiple copies):
>>>>
>>>>- in a return statement in a function with a class return type, when
>>>>the expression is the name of a non-volatile automatic object (other
>>>>than a function or catch-clause parameter) with the same cv-
>>>>unqualified type as the function return type, the copy/move
>>>>operation can be omitted by constructing the automatic object
>>>>directly into the function’s return value
>>>>
>>>>and then p32:
>>>>
>>>>When the criteria for elision of a copy operation are met or would
>>>>be met save for the fact that the source object is a function
>>>>parameter, and the object to be copied is designated by an lvalue,
>>>>overload resolution to select the constructor for the copy is first
>>>>performed as if the object were designated by an rvalue.
>>>>
>>>>The constructor isn't required to be elided in C++11, but the compiler
>>>>is required to use a move constructor instead of a copy constructor,
>>>>if a move constructor is available. So you don't need to use std::move
>>>>on the return value.
>>>>
>>>>And the code above compiles fine with gcc 4.8.5 on CentOS 7 (and even
>>>>with 4.7.1).
>>>>
>>>>So the std::move calls you've added are redundant, and will cause
>>>>-Wredundant-move warnings.
>>>>
>>>>What's the error you were seeing?
>>>
>>>I can reproduce it:
>>>
>>>/home/test/src/gcc/gcc/analyzer/constraint-manager.cc: In member function ‘std::unique_ptr<text_art::widget> ana::equiv_class::make_dump_widget(const text_art::dump_widget_info&, unsigned int) const’:
>>>/home/test/src/gcc/gcc/analyzer/constraint-manager.cc:1179:10: error: cannot bind ‘std::unique_ptr<text_art::tree_widget>’ lvalue to ‘std::unique_ptr<text_art::tree_widget>&&’
>>> return ec_widget;
>>>        ^
>>>
>>>Odd ... I'm looking into it ...
>>
>>Ah, the problem here is that the function's return type is
>>std::unique_ptr<text_art::widget> but the return value is
>>std::unique_ptr<tree_widget> and that requires the converting
>>constructor, not the move constructor.
>>
>>That *did* change after C++11, and isn't supported by G++ until 5.1
>
>That changed with https://cplusplus.github.io/CWG/issues/1579.html

Which for the record was implemented in r5-1576-gfb682f9458c6cf (by
me, which I'd forgotten!)

>which is in C++14 and is a DR against C++11, but of course compilers
>that implement the original C++11 rules don't support that DR.
>
>Arguably, g++ should not give -Wredundant-move warnings here when
>using the -std=c++11 dialect, because it's only redundant if the C++11
>compiler implements that DR.
>
>>Is there a reason that function has to return unique_ptr<widget>
>>instead of unique_ptr<tree_widget>? The conversion to ptr-to-base
>>could happen at the call site (where you always have an rvalue)
>>instead of on the return value.
>
>Changing the return type to match the return value would avoid the
>issue entirely though, as it would be valid with all C++11 compilers.
>
>