From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24835 invoked by alias); 17 Dec 2018 23:30:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 24455 invoked by uid 89); 17 Dec 2018 23:30:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=tree_value, TREE_VALUE, tree_chain, TREE_CHAIN X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Dec 2018 23:30:23 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9A7F2C7A29 for ; Mon, 17 Dec 2018 23:30:15 +0000 (UTC) Received: from ovpn-117-98.phx2.redhat.com (ovpn-117-98.phx2.redhat.com [10.3.117.98]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0B56617B5A; Mon, 17 Dec 2018 23:30:14 +0000 (UTC) Message-ID: <1545089414.4619.239.camel@redhat.com> Subject: Re: [PATCH] v5: C++: more location wrapper nodes (PR c++/43064, PR c++/43486) From: David Malcolm To: Jason Merrill Cc: Jeff Law , gcc-patches@gcc.gnu.org Date: Mon, 17 Dec 2018 23:30:00 -0000 In-Reply-To: <4a0448bb-8f3e-92cb-0e13-f8928632cc0c@redhat.com> References: <1544833064-29934-1-git-send-email-dmalcolm@redhat.com> <4a0448bb-8f3e-92cb-0e13-f8928632cc0c@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg01269.txt.bz2 On Mon, 2018-12-17 at 14:33 -0500, Jason Merrill wrote: > On 12/14/18 7:17 PM, David Malcolm wrote: > > + /* Since default args are effectively part of the function > > type, > > + strip location wrappers here, since otherwise the > > location of > > + one function's default arguments is arbitrarily chosen > > for > > + all functions with similar signature (due to > > canonicalization > > + of function types). */ > > Hmm, looking at this again, why would this happen? I see that > type_list_equal uses == to compare default arguments, so two > function > types with the same default argument but different location wrappers > shouldn't be combined. > > Jason Thanks. I did some digging into this. I added this strip to fix g++.dg/template/defarg6.C but it looks like I was overzealous (the comment is correct, but it's papering over a problem). It turns out that type_list_equal is doing more than just pointer equality; it's hitting the simple_cst_equal part of the && at line 7071: 7063 bool 7064 type_list_equal (const_tree l1, const_tree l2) 7065 { 7066 const_tree t1, t2; 7067 7068 for (t1 = l1, t2 = l2; t1 && t2; t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2)) 7069 if (TREE_VALUE (t1) != TREE_VALUE (t2) 7070 || (TREE_PURPOSE (t1) != TREE_PURPOSE (t2) 7071 && ! (1 == simple_cst_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2)) 7072 && (TREE_TYPE (TREE_PURPOSE (t1)) 7073 == TREE_TYPE (TREE_PURPOSE (t2)))))) 7074 return false; 7075 7076 return t1 == t2; 7077 } What's happening is that there are two different functions with identical types apart from the locations of their (equal) default arguments: both of the TREE_PURPOSEs are NON_LVALUE_EXPR wrappers around a CONST_DECL enum value (at different source locations). simple_cst_equal is stripping the location wrappers here: 7311 if (CONVERT_EXPR_CODE_P (code1) || code1 == NON_LVALUE_EXPR) 7312 { 7313 if (CONVERT_EXPR_CODE_P (code2) 7314 || code2 == NON_LVALUE_EXPR) 7315 return simple_cst_equal (TREE_OPERAND (t1, 0), TREE_OPERAND (t2, 0)); 7316 else 7317 return simple_cst_equal (TREE_OPERAND (t1, 0), t2); 7318 } and thus finds them to be equal; the iteration in type_list_equal continues, and runs out of parameters with t1 == t2 == NULL, and thus returns true, and thus the two function types hash to the same slot, and the two function types get treated as being the same. It's not clear to me yet what the best solution to this is: - should simple_cst_equal regard different source locations as being different? - should function-type hashing use a custom version of type_list_equal when comparing params, and make different source locations of default args be different? - something else? Dave