From patchwork Wed Aug 26 12:30:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 1351916 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=acm.org Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=JbYuvrol; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Bc4v748H8z9sSJ for ; Wed, 26 Aug 2020 22:30:10 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2D2C6386186A; Wed, 26 Aug 2020 12:30:07 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qk1-x731.google.com (mail-qk1-x731.google.com [IPv6:2607:f8b0:4864:20::731]) by sourceware.org (Postfix) with ESMTPS id DB5F9385783F for ; Wed, 26 Aug 2020 12:30:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DB5F9385783F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=acm.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nathanmsidwell@gmail.com Received: by mail-qk1-x731.google.com with SMTP id o196so1083057qke.8 for ; Wed, 26 Aug 2020 05:30:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=XgsleE+k3Q4LxF3soavr8rtOpy8sY+bNEHvTAafkrFM=; b=JbYuvrol7IYooSZ9TG1TPbmY7yauLDcGJf3HSYK0F5jSaWmUglSxtpphqLVrjZYDIz SiB1lnHcZ63mn9kGY11/XPa817Q3CNT+Q1SgP1ua8DFVoYdOUrFNC3p0B2xt0poSWel3 3FJUGzrPeoQDIwdO9Cuhn38XJjMGiRnlzij5vmrBVNzgIl/vNsO2xfl+ZUsQWA3Jey4l 72wywIPS0cSq3AnsbTAbJSERnwSl5gO95M5CMSXE27PcqgTA+r4Sy7OdxLLR3BLWDg9V SF8XNVvnYZfQ3gnMh2qLVvpnJDQzc5kxQZRt8XpE7O0/+9L0YstNxP9R2Iz8iTCI7teK VKkA== 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=XgsleE+k3Q4LxF3soavr8rtOpy8sY+bNEHvTAafkrFM=; b=GcXe2w7KiiD4+oBmogTVstEkcrz3nS9LKIi4AbokvFyWn6NAFzzJpYi3Ts5047PfW+ jSCZrqMTeHqueYw8R1EHJtzbGMsABJCLGTkXvRt6/PGQu0cA9JJ39SQHKConJtnMuz03 AefWEwtzgo1x3P9yiP16LGtWjXn9HakyD0C96+53Dno7EZ/gWLPFr2BkpOIxUixUoisu cp7tvRTPZ6Rm50UWAVYRom161sLEBe7q8qc0+AkLZ4WyhidFZAlYbW/dvEJlexT0gIqg CAKDQ8XV0D+ikqTXR2tu1tZTWbmT1U8mI9/F1HNml0wbYShYZCTzIzvpttAuZlvRmmtH 3dXA== X-Gm-Message-State: AOAM530pI9L4/EsKYr7gsSyR3HXHHRZCIE84SeSiv3fGS5LTDUadrFLM sc6wxWckY0XJxWs5YMUWSZk= X-Google-Smtp-Source: ABdhPJzgLiyu3Ay9puo08mcVPNwmVJ02IV26btDDR5iW3ZURIvMtyTkUW6HDc4SOAGZ/31iq/4yPIg== X-Received: by 2002:a37:498e:: with SMTP id w136mr12984795qka.187.1598445002949; Wed, 26 Aug 2020 05:30:02 -0700 (PDT) Received: from ?IPv6:2620:10d:c0a8:1102:54ca:2b5a:e3fa:ee01? ([2620:10d:c091:480::1:a662]) by smtp.googlemail.com with ESMTPSA id d20sm1499792qkk.84.2020.08.26.05.30.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 26 Aug 2020 05:30:01 -0700 (PDT) To: GCC Patches From: Nathan Sidwell Subject: c++: template operator lookup caching Message-ID: <9741b914-866f-8f29-c9e9-8ba3be40d5b2@acm.org> Date: Wed, 26 Aug 2020 08:30:00 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 Content-Language: en-US X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Jason's fix to retain operator lookups inside dependent lambda functions turns out to be needed on the modules branch for all template functions. Because the context for that lookup no longer exists in imports. There were also a couple of shortcomings, which this patch fixes. (a) we conflate 'we found nothing' and 'we can redo this at instantiation time'. Fixed by making the former produce error_mark_node. That needs a fix in name-lookup to know that finding a binding containing error_mark_node, means 'stop looking' you found nothing. (b) we'd continually do lookups for every operator, if nothing needed to be retained. Fixed by always caching that information, and then dealing with it when pushing the bindings. (c) if what we found was that find by a global namespace lookup, we'd not cache that. But that'd cause us to find decls declared after the template, potentially hiding those we expected to find. So don't do that check. This still retains only recording on lambdas. As the comment says, we could enable for all templates. gcc/cp/ * decl.c (poplevel): A local-binding tree list holds the name in TREE_PURPOSE. * name-lookup.c (update_local_overload): Add id to TREE_PURPOSE. (lookup_name_1): Deal with local-binding error_mark_node marker. (op_unqualified_lookup): Return error_mark_node for 'nothing found'. Retain global binding, check class binding here. (maybe_save_operator_binding): Reimplement to always cache a result. (push_operator_bindings): Deal with 'ignore' marker. gcc/testsuite/ * g++.dg/lookup/operator-1.C: New. * g++.dg/lookup/operator-2.C: New. pushed nathan diff --git c/gcc/cp/decl.c w/gcc/cp/decl.c index d1af63e7300..5e17e4dc4b1 100644 --- c/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -692,8 +692,18 @@ poplevel (int keep, int reverse, int functionbody) /* Remove declarations for all the DECLs in this level. */ for (link = decls; link; link = TREE_CHAIN (link)) { - decl = TREE_CODE (link) == TREE_LIST ? TREE_VALUE (link) : link; - tree name = OVL_NAME (decl); + tree name; + if (TREE_CODE (link) == TREE_LIST) + { + decl = TREE_VALUE (link); + name = TREE_PURPOSE (link); + gcc_checking_assert (name); + } + else + { + decl = link; + name = DECL_NAME (decl); + } /* Remove the binding. */ if (TREE_CODE (decl) == LABEL_DECL) diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index c68ea09e610..3c2ddc197e6 100644 --- c/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -2305,7 +2305,7 @@ update_local_overload (cxx_binding *binding, tree newval) if (*d == binding->value) { /* Stitch new list node in. */ - *d = tree_cons (NULL_TREE, NULL_TREE, TREE_CHAIN (*d)); + *d = tree_cons (DECL_NAME (*d), NULL_TREE, TREE_CHAIN (*d)); break; } else if (TREE_CODE (*d) == TREE_LIST && TREE_VALUE (*d) == binding->value) @@ -3239,7 +3239,7 @@ push_local_binding (tree id, tree decl, bool is_using) if (TREE_CODE (decl) == OVERLOAD || is_using) /* We must put the OVERLOAD or using into a TREE_LIST since we cannot use the decl's chain itself. */ - decl = build_tree_list (NULL_TREE, decl); + decl = build_tree_list (id, decl); /* And put DECL on the list of things declared by the current binding level. */ @@ -6539,13 +6539,20 @@ lookup_name_1 (tree name, LOOK_where where, LOOK_want want) gcc_assert (TREE_CODE (binding) == TYPE_DECL); continue; } + + /* The saved lookups for an operator record 'nothing + found' as error_mark_node. We need to stop the search + here, but not return the error mark node. */ + if (binding == error_mark_node) + binding = NULL_TREE; + val = binding; - break; + goto found; } } /* Now lookup in namespace scopes. */ - if (!val && bool (where & LOOK_where::NAMESPACE)) + if (bool (where & LOOK_where::NAMESPACE)) { name_lookup lookup (name, want); if (lookup.search_unqualified @@ -6553,6 +6560,8 @@ lookup_name_1 (tree name, LOOK_where where, LOOK_want want) val = lookup.value; } + found:; + /* If we have a single function from a using decl, pull it out. */ if (val && TREE_CODE (val) == OVERLOAD && !really_overloaded_fn (val)) val = OVL_FUNCTION (val); @@ -7598,23 +7607,38 @@ op_unqualified_lookup (tree fnname) cp_binding_level *l = binding->scope; while (l && !l->this_entity) l = l->level_chain; + if (l && uses_template_parms (l->this_entity)) /* Don't preserve decls from an uninstantiated template, wait until that template is instantiated. */ return NULL_TREE; } + tree fns = lookup_name (fnname); - if (fns && fns == get_global_binding (fnname)) - /* The instantiation can find these. */ - return NULL_TREE; + if (!fns) + /* Remember we found nothing! */ + return error_mark_node; + + tree d = is_overloaded_fn (fns) ? get_first_fn (fns) : fns; + if (DECL_CLASS_SCOPE_P (d)) + /* We don't need to remember class-scope functions or declarations, + normal unqualified lookup will find them again. */ + fns = NULL_TREE; + return fns; } /* E is an expression representing an operation with dependent type, so we don't know yet whether it will use the built-in meaning of the operator or a - function. Remember declarations of that operator in scope. */ + function. Remember declarations of that operator in scope. + + We then inject a fake binding of that lookup into the + instantiation's parameter scope. This approach fails if the user + has different using declarations or directives in different local + binding of the current function from whence we need to do lookups + (we'll cache what we see on the first lookup). */ -const char *const op_bind_attrname = "operator bindings"; +static const char *const op_bind_attrname = "operator bindings"; void maybe_save_operator_binding (tree e) @@ -7622,13 +7646,14 @@ maybe_save_operator_binding (tree e) /* This is only useful in a generic lambda. */ if (!processing_template_decl) return; + tree cfn = current_function_decl; if (!cfn) return; - /* Let's only do this for generic lambdas for now, we could do it for all - function templates if we wanted to. */ - if (!current_lambda_expr()) + /* Do this for lambdas and code that will emit a CMI. In a module's + GMF we don't yet know whether there will be a CMI. */ + if (!current_lambda_expr ()) return; tree fnname = ovl_op_identifier (false, TREE_CODE (e)); @@ -7636,32 +7661,22 @@ maybe_save_operator_binding (tree e) return; tree attributes = DECL_ATTRIBUTES (cfn); - tree attr = lookup_attribute (op_bind_attrname, attributes); - tree bindings = NULL_TREE; - tree fns = NULL_TREE; - if (attr) + tree op_attr = lookup_attribute (op_bind_attrname, attributes); + if (!op_attr) { - bindings = TREE_VALUE (attr); - if (tree elt = purpose_member (fnname, bindings)) - fns = TREE_VALUE (elt); + op_attr = tree_cons (get_identifier (op_bind_attrname), + NULL_TREE, attributes); + DECL_ATTRIBUTES (cfn) = op_attr; } - if (!fns && (fns = op_unqualified_lookup (fnname))) + tree op_bind = purpose_member (fnname, TREE_VALUE (op_attr)); + if (!op_bind) { - tree d = is_overloaded_fn (fns) ? get_first_fn (fns) : fns; - if (DECL_P (d) && DECL_CLASS_SCOPE_P (d)) - /* We don't need to remember class-scope functions or declarations, - normal unqualified lookup will find them again. */ - return; + tree fns = op_unqualified_lookup (fnname); - bindings = tree_cons (fnname, fns, bindings); - if (attr) - TREE_VALUE (attr) = bindings; - else - DECL_ATTRIBUTES (cfn) - = tree_cons (get_identifier (op_bind_attrname), - bindings, - attributes); + /* Always record, so we don't keep looking for this + operator. */ + TREE_VALUE (op_attr) = tree_cons (fnname, fns, TREE_VALUE (op_attr)); } } @@ -7684,11 +7699,11 @@ push_operator_bindings () if (tree attr = lookup_attribute (op_bind_attrname, DECL_ATTRIBUTES (decl1))) for (tree binds = TREE_VALUE (attr); binds; binds = TREE_CHAIN (binds)) - { - tree name = TREE_PURPOSE (binds); - tree val = TREE_VALUE (binds); - push_local_binding (name, val, /*using*/true); - } + if (tree val = TREE_VALUE (binds)) + { + tree name = TREE_PURPOSE (binds); + push_local_binding (name, val, /*using*/true); + } } #include "gt-cp-name-lookup.h" diff --git c/gcc/testsuite/g++.dg/lookup/operator-1.C w/gcc/testsuite/g++.dg/lookup/operator-1.C new file mode 100644 index 00000000000..98ef376fef5 --- /dev/null +++ w/gcc/testsuite/g++.dg/lookup/operator-1.C @@ -0,0 +1,20 @@ +// { dg-do compile { target c++11 } } + +template bool Foo (T x) +{ + return [](T x) + { return !x; }(x); // { dg-error "no match for 'operator!'" } +} + +namespace X +{ +struct S {}; +} + +// not found by adl :) +bool operator! (X::S); + +int main () +{ + Foo (X::S{}); +} diff --git c/gcc/testsuite/g++.dg/lookup/operator-2.C w/gcc/testsuite/g++.dg/lookup/operator-2.C new file mode 100644 index 00000000000..46d1d19daf2 --- /dev/null +++ w/gcc/testsuite/g++.dg/lookup/operator-2.C @@ -0,0 +1,23 @@ +// { dg-do compile { target c++11 } } + +struct R{}; +bool operator! (R); // { dg-message "candidate:" } + +template bool Foo (T x) +{ + return [](T x) + { return !x; }(x); // { dg-error "no match for 'operator!'" } +} + +namespace X +{ +struct S {}; +} + +// not found by adl :) +bool operator! (X::S); + +int main () +{ + Foo (X::S{}); +}