diff mbox series

[analyzer] Restore bootstrap with g++ 4.8.

Message ID 026701dab90a$2cae4180$860ac480$@nextmovesoftware.com
State New
Headers show
Series [analyzer] Restore bootstrap with g++ 4.8. | expand

Commit Message

Roger Sayle June 7, 2024, 6:40 p.m. UTC
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.

Bootstrapped on x86_64-pc-linux-gnu using both gcc 4.8.5 (system) and
gcc 10.2.1 (using "scl enable devetoolset-10") as host compilers.
Ok for mainline?


2024-06-07  Roger Sayle  <roger@nextmovesoftware.com>

gcc/analyzer/ChangeLog
        * constraint-manager.cc (equiv_class::make_dump_widget): Use
        std::move to return a std::unique_ptr.
        (bounded_ranges_constraint::make_dump_widget): Likewise.
        (constraint_manager::make_dump_widget): Likewise.
        * program_state.cc (sm_state_map::make_dump_widget): Likewise.
        (program_state::make_dump_widget): Likewise.
        * region-model.cc (region_to_value_map::make_dump_widget): Likewise.
        (region_model::make_dump_widget): Likewise.
        * region.cc (region::make_dump_widget): Likewise.
        * store.cc (binding_cluster::make_dump_widget): Likewise.
        (store::make_dump_widget): Likewise.
        * svalue.cc (svalue::make_dump_widget): Likewise.

Thanks in advance,
Roger
--

Comments

David Malcolm June 7, 2024, 8:18 p.m. UTC | #1
On Fri, 2024-06-07 at 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.
> 
> Bootstrapped on x86_64-pc-linux-gnu using both gcc 4.8.5 (system) and
> gcc 10.2.1 (using "scl enable devetoolset-10") as host compilers.
> Ok for mainline?

Yes, thanks.  Sorry for the breakage.

Dave

> 
> 
> 2024-06-07  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/analyzer/ChangeLog
>         * constraint-manager.cc (equiv_class::make_dump_widget): Use
>         std::move to return a std::unique_ptr.
>         (bounded_ranges_constraint::make_dump_widget): Likewise.
>         (constraint_manager::make_dump_widget): Likewise.
>         * program_state.cc (sm_state_map::make_dump_widget):
> Likewise.
>         (program_state::make_dump_widget): Likewise.
>         * region-model.cc (region_to_value_map::make_dump_widget):
> Likewise.
>         (region_model::make_dump_widget): Likewise.
>         * region.cc (region::make_dump_widget): Likewise.
>         * store.cc (binding_cluster::make_dump_widget): Likewise.
>         (store::make_dump_widget): Likewise.
>         * svalue.cc (svalue::make_dump_widget): Likewise.
> 
> Thanks in advance,
> Roger
> --
>
Jonathan Wakely June 14, 2024, 9:36 a.m. UTC | #2
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?


>Bootstrapped on x86_64-pc-linux-gnu using both gcc 4.8.5 (system) and
>gcc 10.2.1 (using "scl enable devetoolset-10") as host compilers.
>Ok for mainline?
>
>
>2024-06-07  Roger Sayle  <roger@nextmovesoftware.com>
>
>gcc/analyzer/ChangeLog
>        * constraint-manager.cc (equiv_class::make_dump_widget): Use
>        std::move to return a std::unique_ptr.
>        (bounded_ranges_constraint::make_dump_widget): Likewise.
>        (constraint_manager::make_dump_widget): Likewise.
>        * program_state.cc (sm_state_map::make_dump_widget): Likewise.
>        (program_state::make_dump_widget): Likewise.
>        * region-model.cc (region_to_value_map::make_dump_widget): Likewise.
>        (region_model::make_dump_widget): Likewise.
>        * region.cc (region::make_dump_widget): Likewise.
>        * store.cc (binding_cluster::make_dump_widget): Likewise.
>        (store::make_dump_widget): Likewise.
>        * svalue.cc (svalue::make_dump_widget): Likewise.
>
>Thanks in advance,
>Roger
>--
>

>diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
>index 707385d..883f33b 100644
>--- a/gcc/analyzer/constraint-manager.cc
>+++ b/gcc/analyzer/constraint-manager.cc
>@@ -1176,7 +1176,7 @@ equiv_class::make_dump_widget (const text_art::dump_widget_info &dwi,
>       ec_widget->add_child (tree_widget::make (dwi, &pp));
>     }
>
>-  return ec_widget;
>+  return std::move (ec_widget);
> }
>
> /* Generate a hash value for this equiv_class.
>@@ -1500,7 +1500,7 @@ make_dump_widget (const text_art::dump_widget_info &dwi) const
>     (tree_widget::from_fmt (dwi, nullptr,
> 			    "ec%i bounded ranges", m_ec_id.as_int ()));
>   m_ranges->add_to_dump_widget (*brc_widget.get (), dwi);
>-  return brc_widget;
>+  return std::move (brc_widget);
> }
>
> bool
>@@ -1853,7 +1853,7 @@ constraint_manager::make_dump_widget (const text_art::dump_widget_info &dwi) con
>   if (cm_widget->get_num_children () == 0)
>     return nullptr;
>
>-  return cm_widget;
>+  return std::move (cm_widget);
> }
>
> /* Attempt to add the constraint LHS OP RHS to this constraint_manager.
>diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
>index dc2d4bd..efaf569 100644
>--- a/gcc/analyzer/program-state.cc
>+++ b/gcc/analyzer/program-state.cc
>@@ -382,7 +382,7 @@ sm_state_map::make_dump_widget (const text_art::dump_widget_info &dwi,
>       state_widget->add_child (tree_widget::make (dwi, pp));
>     }
>
>-  return state_widget;
>+  return std::move (state_widget);
> }
>
> /* Return true if no states have been set within this map
>@@ -1247,7 +1247,7 @@ program_state::make_dump_widget (const text_art::dump_widget_info &dwi) const
> 	state_widget->add_child (smap->make_dump_widget (dwi, m_region_model));
>   }
>
>-  return state_widget;
>+  return std::move (state_widget);
> }
>
> /* Update this program_state to reflect a top-level call to FUN.
>diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
>index d142d85..4fbc970 100644
>--- a/gcc/analyzer/region-model.cc
>+++ b/gcc/analyzer/region-model.cc
>@@ -288,7 +288,7 @@ make_dump_widget (const text_art::dump_widget_info &dwi) const
>       sval->dump_to_pp (pp, true);
>       w->add_child (text_art::tree_widget::make (dwi, pp));
>     }
>-  return w;
>+  return std::move (w);
> }
>
> /* Attempt to merge THIS with OTHER, writing the result
>@@ -556,7 +556,7 @@ region_model::make_dump_widget (const text_art::dump_widget_info &dwi) const
> 			       m_mgr->get_store_manager ()));
>   model_widget->add_child (m_constraints->make_dump_widget (dwi));
>   model_widget->add_child (m_dynamic_extents.make_dump_widget (dwi));
>-  return model_widget;
>+  return std::move (model_widget);
> }
>
> /* Assert that this object is valid.  */
>diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
>index 71bae97..050feb6 100644
>--- a/gcc/analyzer/region.cc
>+++ b/gcc/analyzer/region.cc
>@@ -1119,7 +1119,7 @@ region::make_dump_widget (const text_art::dump_widget_info &dwi,
>   if (m_parent)
>     w->add_child (m_parent->make_dump_widget (dwi, "parent"));
>
>-  return w;
>+  return std::move (w);
> }
>
> void
>diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
>index d14cfa3..b20bc29 100644
>--- a/gcc/analyzer/store.cc
>+++ b/gcc/analyzer/store.cc
>@@ -1489,7 +1489,7 @@ binding_cluster::make_dump_widget (const text_art::dump_widget_info &dwi,
>
>       m_map.add_to_tree_widget (*cluster_widget, dwi);
>
>-      return cluster_widget;
>+      return std::move (cluster_widget);
>     }
> }
>
>@@ -2766,7 +2766,7 @@ store::make_dump_widget (const text_art::dump_widget_info &dwi,
>       store_widget->add_child (std::move (parent_reg_widget));
>     }
>
>-  return store_widget;
>+  return std::move (store_widget);
> }
>
> /* Get any svalue bound to REG, or NULL.  */
>diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
>index f1fd21e..b67780a 100644
>--- a/gcc/analyzer/svalue.cc
>+++ b/gcc/analyzer/svalue.cc
>@@ -252,7 +252,7 @@ svalue::make_dump_widget (const text_art::dump_widget_info &dwi,
>
>   add_dump_widget_children (*w, dwi);
>
>-  return w;
>+  return std::move (w);
> }
>
> /* If this svalue is a constant_svalue, return the underlying tree constant.
Jonathan Wakely June 14, 2024, 10:26 a.m. UTC | #3
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 ...
Jonathan Wakely June 14, 2024, 10:37 a.m. UTC | #4
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

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.
Jonathan Wakely June 14, 2024, 10:42 a.m. UTC | #5
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 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.
Jonathan Wakely June 14, 2024, 10:44 a.m. UTC | #6
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.
>
>
diff mbox series

Patch

diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 707385d..883f33b 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -1176,7 +1176,7 @@  equiv_class::make_dump_widget (const text_art::dump_widget_info &dwi,
       ec_widget->add_child (tree_widget::make (dwi, &pp));
     }
 
-  return ec_widget;
+  return std::move (ec_widget);
 }
 
 /* Generate a hash value for this equiv_class.
@@ -1500,7 +1500,7 @@  make_dump_widget (const text_art::dump_widget_info &dwi) const
     (tree_widget::from_fmt (dwi, nullptr,
 			    "ec%i bounded ranges", m_ec_id.as_int ()));
   m_ranges->add_to_dump_widget (*brc_widget.get (), dwi);
-  return brc_widget;
+  return std::move (brc_widget);
 }
 
 bool
@@ -1853,7 +1853,7 @@  constraint_manager::make_dump_widget (const text_art::dump_widget_info &dwi) con
   if (cm_widget->get_num_children () == 0)
     return nullptr;
 
-  return cm_widget;
+  return std::move (cm_widget);
 }
 
 /* Attempt to add the constraint LHS OP RHS to this constraint_manager.
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index dc2d4bd..efaf569 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -382,7 +382,7 @@  sm_state_map::make_dump_widget (const text_art::dump_widget_info &dwi,
       state_widget->add_child (tree_widget::make (dwi, pp));
     }
 
-  return state_widget;
+  return std::move (state_widget);
 }
 
 /* Return true if no states have been set within this map
@@ -1247,7 +1247,7 @@  program_state::make_dump_widget (const text_art::dump_widget_info &dwi) const
 	state_widget->add_child (smap->make_dump_widget (dwi, m_region_model));
   }
 
-  return state_widget;
+  return std::move (state_widget);
 }
 
 /* Update this program_state to reflect a top-level call to FUN.
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index d142d85..4fbc970 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -288,7 +288,7 @@  make_dump_widget (const text_art::dump_widget_info &dwi) const
       sval->dump_to_pp (pp, true);
       w->add_child (text_art::tree_widget::make (dwi, pp));
     }
-  return w;
+  return std::move (w);
 }
 
 /* Attempt to merge THIS with OTHER, writing the result
@@ -556,7 +556,7 @@  region_model::make_dump_widget (const text_art::dump_widget_info &dwi) const
 			       m_mgr->get_store_manager ()));
   model_widget->add_child (m_constraints->make_dump_widget (dwi));
   model_widget->add_child (m_dynamic_extents.make_dump_widget (dwi));
-  return model_widget;
+  return std::move (model_widget);
 }
 
 /* Assert that this object is valid.  */
diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index 71bae97..050feb6 100644
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -1119,7 +1119,7 @@  region::make_dump_widget (const text_art::dump_widget_info &dwi,
   if (m_parent)
     w->add_child (m_parent->make_dump_widget (dwi, "parent"));
 
-  return w;
+  return std::move (w);
 }
 
 void
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index d14cfa3..b20bc29 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -1489,7 +1489,7 @@  binding_cluster::make_dump_widget (const text_art::dump_widget_info &dwi,
 
       m_map.add_to_tree_widget (*cluster_widget, dwi);
 
-      return cluster_widget;
+      return std::move (cluster_widget);
     }
 }
 
@@ -2766,7 +2766,7 @@  store::make_dump_widget (const text_art::dump_widget_info &dwi,
       store_widget->add_child (std::move (parent_reg_widget));
     }
 
-  return store_widget;
+  return std::move (store_widget);
 }
 
 /* Get any svalue bound to REG, or NULL.  */
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index f1fd21e..b67780a 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -252,7 +252,7 @@  svalue::make_dump_widget (const text_art::dump_widget_info &dwi,
 
   add_dump_widget_children (*w, dwi);
 
-  return w;
+  return std::move (w);
 }
 
 /* If this svalue is a constant_svalue, return the underlying tree constant.