From patchwork Wed May 17 12:44:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 763508 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wSYtD17vLz9s7w for ; Wed, 17 May 2017 22:44:35 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="u14isJAT"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=IsgHhndVKs6BYMwlLsNa4S2H6posYK3ZacKdnWJeCI61k76sTk yO5DNd6di2t5AiD6HrTu2R9vsIUP5CB2Pk+yasDqVCEO8EJ+NPk6fxjlvfAnlIoJ iV8oPI9PrfEXn+98GKA9IOhqEotS3RDVSvHyiC0vldsOs2U5l5VuzJz1E= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=T1+UlHRDywNTKRisrA4GAKwm24U=; b=u14isJATs4D/T2Lifc64 D9SnPNZvhs47wA1EGQyrEwIHbD6d3NKnRJk+boqP0WZjs22qq+MjXt0ibSFoI+m4 XZIyuvAA/5KFZrzVBtLU+1OnYzCOx+IoKOjclpaFmvWEo45OvIVnJY4/aDvjuOjA ew/SPg/r6cvbMx2cgOp7RcQ= Received: (qmail 39215 invoked by alias); 17 May 2017 12:44:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 39198 invoked by uid 89); 17 May 2017 12:44:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=exports, 11327, trail X-HELO: mail-yw0-f178.google.com Received: from mail-yw0-f178.google.com (HELO mail-yw0-f178.google.com) (209.85.161.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 17 May 2017 12:44:19 +0000 Received: by mail-yw0-f178.google.com with SMTP id 203so5093416ywe.0 for ; Wed, 17 May 2017 05:44:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:to:from:subject:message-id:date :user-agent:mime-version:content-language; bh=y7S6Nmx3XvwGE1mslpPm8u7T3pOljnNZss7nyRnZP7M=; b=CGMkI16d+lYoc0kjl0dxF3Yb3bRVav173CTLI2BjmRHSPg2se7rkrQbsLZYTJTdMc2 vZsUiho81cnVSdZH1TolpjgpGaDbgvkL34eWk+Kwfy0JENVT5MGr2X25teMrpDKr2Z8T 760yfGuDa7BJ5SB0husFqkkhjFpAKNH8P8Dq9/vdMJoG/6zCXBwYKHCDPSzON8eCz5/a 6dPv3mPNtO0W4Fo0b4B0AGh1qRnsqjvqi26jQHlnjyC09lUEPWqBJn+5/e3I6ndcWqlU rU5mMn3K1TMsURUcipLa/vZt0bku/LA5MkVH1m3llmxp+E6gmAfL81Mo7TbjJTgrKrRn WZtg== X-Gm-Message-State: AODbwcBZLYdHIYk7GQMuK1ldwqI8wpZSWWQ2yEaOPj0AMp6bHHy05bkX EBKtlrzWNx8YVA== X-Received: by 10.129.141.67 with SMTP id w3mr2638067ywj.64.1495025061064; Wed, 17 May 2017 05:44:21 -0700 (PDT) Received: from ?IPv6:2620:10d:c0a3:20fb:f6d0:5ac5:64cd:f102? ([2620:10d:c091:200::7:4a1a]) by smtp.googlemail.com with ESMTPSA id z19sm861047ywj.52.2017.05.17.05.44.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 17 May 2017 05:44:20 -0700 (PDT) To: GCC Patches From: Nathan Sidwell Subject: [C++ PATCH] OVERLOAD iterators #5 Message-ID: Date: Wed, 17 May 2017 08:44:18 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 This patch fixes up add_method. There are two things going on here 1) a new ovl_insert worker to add a new decl to an existing overload set. This is no longer simply prepending the new decl, as we keep the set ordered. The eventual ordering will be hidden->using->regular->exported. But at this point we just keep the using decls before the regular ones. Hidden handling will be later and of course exports is a modules thing. (hidden fns are both friend injections and anticipated builtins) 2) a remove_node worker, rather than have users directly stitch out the node. Both these behaviours mean that there's an issue with overload lookups that are stashed in uninstantiated templates. If we can subsequently go modifying that lookup, a later instantiation may see different functions than it should do. We already have this problem because of #2 (users directly stitching out), and #1 now introduces another way it can happen. This patch does not address that problem. I have further patches to rectify this. There are no new testcase failures because of this problem (I have new testcases too). nathan 2017-05-17 Nathan Sidwell * cp-tree.h (ovl_iterator::using_p): New predicate. (ovl_iterator::remove_node): New worker. (ovl_insert): Declare. * tree.c (ovl_insert): New. (ovl_iterator::remove_node): New. * class.c (add_method): Use ovl_iterator, ovl_insert. (clone_function_decl): Fix description. (clone_constructors_and_destructors): Use ovl_iterator. Index: class.c =================================================================== --- class.c (revision 248144) +++ class.c (working copy) @@ -1010,7 +1010,6 @@ bool add_method (tree type, tree method, bool via_using) { unsigned slot; - tree overload; bool template_conv_p = false; bool conv_p; vec *method_vec; @@ -1059,7 +1058,7 @@ add_method (tree type, tree method, bool vec_safe_iterate (method_vec, slot, &m); ++slot) { - m = OVL_CURRENT (m); + m = OVL_FIRST (m); if (template_conv_p) { if (TREE_CODE (m) == TEMPLATE_DECL @@ -1083,24 +1082,23 @@ add_method (tree type, tree method, bool current_fns = insert_p ? NULL_TREE : (*method_vec)[slot]; /* Check to see if we've already got this method. */ - for (tree *p = ¤t_fns; *p; ) + for (ovl_iterator iter (current_fns); iter; ++iter) { - tree fns = *p; - tree fn = OVL_CURRENT (fns); + tree fn = *iter; tree fn_type; tree method_type; tree parms1; tree parms2; if (TREE_CODE (fn) != TREE_CODE (method)) - goto cont; + continue; /* Two using-declarations can coexist, we'll complain about ambiguity in overload resolution. */ - if (via_using && TREE_CODE (fns) == OVERLOAD && OVL_USED (fns) + if (via_using && iter.using_p () /* Except handle inherited constructors specially. */ && ! DECL_CONSTRUCTOR_P (fn)) - goto cont; + continue; /* [over.load] Member function declarations with the same name and the same parameter types cannot be @@ -1134,7 +1132,7 @@ add_method (tree type, tree method, bool == FUNCTION_REF_QUALIFIED (method_type)) && (type_memfn_quals (fn_type) != type_memfn_quals (method_type) || type_memfn_rqual (fn_type) != type_memfn_rqual (method_type))) - goto cont; + continue; /* For templates, the return type and template parameters must be identical. */ @@ -1143,7 +1141,7 @@ add_method (tree type, tree method, bool TREE_TYPE (method_type)) || !comp_template_parms (DECL_TEMPLATE_PARMS (fn), DECL_TEMPLATE_PARMS (method)))) - goto cont; + continue; if (! DECL_STATIC_FUNCTION_P (fn)) parms1 = TREE_CHAIN (parms1); @@ -1187,8 +1185,9 @@ add_method (tree type, tree method, bool mangle_decl (method); } cgraph_node::record_function_versions (fn, method); - goto cont; + continue; } + if (DECL_INHERITED_CTOR (method)) { if (DECL_INHERITED_CTOR (fn)) @@ -1202,7 +1201,7 @@ add_method (tree type, tree method, bool /* Inheriting the same constructor along different paths, combine them. */ SET_DECL_INHERITED_CTOR - (fn, ovl_cons (DECL_INHERITED_CTOR (method), + (fn, ovl_make (DECL_INHERITED_CTOR (method), DECL_INHERITED_CTOR (fn))); /* And discard the new one. */ return false; @@ -1210,7 +1209,7 @@ add_method (tree type, tree method, bool else /* Inherited ctors can coexist until overload resolution. */ - goto cont; + continue; } error_at (DECL_SOURCE_LOCATION (method), "%q#D", method); @@ -1228,8 +1227,8 @@ add_method (tree type, tree method, bool else if (flag_new_inheriting_ctors && DECL_INHERITED_CTOR (fn)) { - /* Hide the inherited constructor. */ - *p = OVL_NEXT (fns); + /* Remove the inherited constructor. */ + current_fns = iter.remove_node (current_fns); continue; } else @@ -1238,33 +1237,19 @@ add_method (tree type, tree method, bool error ("with %q+#D", fn); return false; } - } - - cont: - if (TREE_CODE (fns) == OVERLOAD) - p = &OVL_CHAIN (fns); - else - break; } /* A class should never have more than one destructor. */ if (current_fns && DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (method)) return false; - /* Add the new binding. */ - if (via_using) - { - overload = ovl_cons (method, current_fns); - OVL_USED (overload) = true; - } - else - overload = build_overload (method, current_fns); + current_fns = ovl_insert (method, current_fns, via_using); if (conv_p) TYPE_HAS_CONVERSION (type) = 1; else if (slot >= CLASSTYPE_FIRST_CONVERSION_SLOT && !complete_p) - push_class_level_binding (DECL_NAME (method), overload); + push_class_level_binding (DECL_NAME (method), current_fns); if (insert_p) { @@ -1279,13 +1264,13 @@ add_method (tree type, tree method, bool if (reallocated) CLASSTYPE_METHOD_VEC (type) = method_vec; if (slot == method_vec->length ()) - method_vec->quick_push (overload); + method_vec->quick_push (current_fns); else - method_vec->quick_insert (slot, overload); + method_vec->quick_insert (slot, current_fns); } else /* Replace the current slot. */ - (*method_vec)[slot] = overload; + (*method_vec)[slot] = current_fns; return true; } @@ -4873,8 +4858,7 @@ decl_cloned_function_p (const_tree decl, /* Produce declarations for all appropriate clones of FN. If UPDATE_METHODS is true, the clones are added to the - CLASTYPE_METHOD_VEC. VIA_USING indicates whether these are cloning - decls brought in via using declarations (i.e. inheriting ctors). */ + CLASSTYPE_METHOD_VEC. */ void clone_function_decl (tree fn, bool update_methods) @@ -5017,8 +5001,6 @@ adjust_clone_args (tree decl) static void clone_constructors_and_destructors (tree t) { - tree fns; - /* If for some reason we don't have a CLASSTYPE_METHOD_VEC, we bail out now. */ if (!CLASSTYPE_METHOD_VEC (t)) @@ -5026,10 +5008,10 @@ clone_constructors_and_destructors (tree /* While constructors can be via a using declaration, at this point we no longer need to know that. */ - for (fns = CLASSTYPE_CONSTRUCTORS (t); fns; fns = OVL_NEXT (fns)) - clone_function_decl (OVL_CURRENT (fns), /*update_methods=*/true); - for (fns = CLASSTYPE_DESTRUCTORS (t); fns; fns = OVL_NEXT (fns)) - clone_function_decl (OVL_CURRENT (fns), /*update_methods=*/true); + for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t)); iter; ++iter) + clone_function_decl (*iter, /*update_methods=*/true); + for (ovl_iterator iter (CLASSTYPE_DESTRUCTORS (t)); iter; ++iter) + clone_function_decl (*iter, /*update_methods=*/true); } /* Deduce noexcept for a destructor DTOR. */ Index: cp-tree.h =================================================================== --- cp-tree.h (revision 248144) +++ cp-tree.h (working copy) @@ -680,6 +680,22 @@ class ovl_iterator return fn; } + + public: + /* Whether this overload was introduced by a using decl. */ + bool using_p () const + { + return TREE_CODE (ovl) == OVERLOAD && OVL_USED (ovl); + } + tree remove_node (tree head) + { + return remove_node (head, ovl); + } + + private: + /* We make this a static function to avoid the address of the + iterator escaping the local context. */ + static tree remove_node (tree head, tree node); }; /* Iterator over a (potentially) 2 dimensional overload, which is @@ -6768,6 +6784,8 @@ extern tree build_ref_qualified_type (t inline tree ovl_first (tree) ATTRIBUTE_PURE; extern tree ovl_make (tree fn, tree next = NULL_TREE); +extern tree ovl_insert (tree fn, tree maybe_ovl, + bool using_p = false); extern tree lookup_add (tree lookup, tree ovl); extern int is_overloaded_fn (tree); extern tree dependent_name (tree); Index: tree.c =================================================================== --- tree.c (revision 248144) +++ tree.c (working copy) @@ -2124,6 +2124,65 @@ ovl_make (tree fn, tree next) return result; } +/* Add FN to the (potentially NULL) overload set OVL. USING_P is + true, if FN is via a using declaration. Overloads are ordered as + using, regular. */ + +tree +ovl_insert (tree fn, tree maybe_ovl, bool using_p) +{ + int weight = using_p; + + tree result = NULL_TREE; + tree insert_after = NULL_TREE; + + /* Find insertion point. */ + while (maybe_ovl && TREE_CODE (maybe_ovl) == OVERLOAD + && (weight < OVL_USED (maybe_ovl))) + { + if (!result) + result = maybe_ovl; + insert_after = maybe_ovl; + maybe_ovl = OVL_CHAIN (maybe_ovl); + } + + tree trail = fn; + if (maybe_ovl || using_p || TREE_CODE (fn) == TEMPLATE_DECL) + { + trail = ovl_make (fn, maybe_ovl); + if (using_p) + OVL_USED (trail) = true; + } + + if (insert_after) + { + TREE_CHAIN (insert_after) = trail; + TREE_TYPE (insert_after) = unknown_type_node; + } + else + result = trail; + + return result; +} + +/* NODE is on the overloads of OVL. Remove it. */ + +tree +ovl_iterator::remove_node (tree overload, tree node) +{ + tree *slot = &overload; + while (*slot != node) + slot = &OVL_CHAIN (*slot); + + /* Stitch out NODE. We don't have to worry about now making a + singleton overload (and consequently maybe setting its type), + because all uses of this function will be followed by inserting a + new node that must follow the place we've cut this out from. */ + *slot = OVL_CHAIN (node); + + return overload; +} + /* Add a potential overload into a lookup set. */ tree