From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id E0A9A3882176 for ; Fri, 14 Jun 2024 09:36:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E0A9A3882176 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E0A9A3882176 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718357795; cv=none; b=C91AFyEbe0e7IkmkM0MKGou3LsS237WHYSDDAsuecrUDsv+CpLpQbv95yNfdHN0pzc8SgByikbIcVqp1gEJR65OpmCfOK9c9yAr08NVvwg0wP6KK3UEM0zUf3ywdBIfWH/88ftFnkEp/tyerxkJ4Aumnvtt7nC6ZUJL3CLXpCEs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718357795; c=relaxed/simple; bh=R8KMLIzWKS6bmhwi+LH/xC2uUb0JXY5pWfJYKtD/kwU=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=UVJD2p0cPaHpLHVvH4XQMhxsOutfOgiu/WMYLX+GBXH7rUqhY2TJy1p+8mz2lMFwcN7pKA2a5xC4+RenwTHwtrdaW6gcDmovZ7VHi4D8147xVmnltUgPjUjW7k77MjOZe19X1oGp2JgzxU/Vk26rQJpuFV7/bbAwdId9JD3WYAo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718357792; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0urTx+mjLbIZ3Ex2yI0gZ4b1eg2srka3dPRvmbEBwGA=; b=CS3UgKC+sJW6FXQ8KKjOIjCchDmfD/WGpGDC76JFqOlKRAKv/WHOcw3BGyf1Tk1bKZDclu CNoFb6omC8dq5/mk5wWESPeymg5bsLA0sb2WdfvDTxDwl8wsTNZFK9LXkJKk4tprTtUcd+ k+LBbqtt1hyXgGo8H2vOafCkxviOJ9Q= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-44-3hEeCJbSP_akh2MpmWLUSA-1; Fri, 14 Jun 2024 05:36:30 -0400 X-MC-Unique: 3hEeCJbSP_akh2MpmWLUSA-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 653F61956086; Fri, 14 Jun 2024 09:36:29 +0000 (UTC) Received: from localhost (unknown [10.39.192.128]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 7723E19560AA; Fri, 14 Jun 2024 09:36:27 +0000 (UTC) Date: Fri, 14 Jun 2024 10:36:25 +0100 From: Jonathan Wakely To: Roger Sayle Cc: gcc-patches@gcc.gnu.org, 'David Malcolm' Subject: Re: [analyzer PATCH] Restore bootstrap with g++ 4.8. Message-ID: References: <026701dab90a$2cae4180$860ac480$@nextmovesoftware.com> MIME-Version: 1.0 In-Reply-To: <026701dab90a$2cae4180$860ac480$@nextmovesoftware.com> X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 std::unique_ptr f() { std::unique_ptr 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 > >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.