From patchwork Fri Jan 13 13:33:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 91426 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp191880qgi; Fri, 13 Jan 2017 05:33:49 -0800 (PST) X-Received: by 10.98.5.2 with SMTP id 2mr9161063pff.77.1484314429902; Fri, 13 Jan 2017 05:33:49 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id t5si12750536pgj.171.2017.01.13.05.33.49 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Jan 2017 05:33:49 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-446087-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-446087-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-446087-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=ybLgZ7AI66FsFat+f nnTEB1NpVWcMaCuZ215TMxnpppXSAvmQ/511UW937Auv2BqgrQ771Vo/XBhAHzwk rLlVfriDJ/7PERTQUfQ0W2GAr2Y53WxFSTMgkb35TeqYHwIxF15bd1/SOv4YyGxd iQ52plLi5W4x4qW+ObNbd9cl5I= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=VY3Ih9/xB4q1keM7TU1vhMK rAs4=; b=C50v8f9adfFCRroDyTg3PQ0qU8jDdkPyAWp8ph7ImMIm3g3QeLLYxdF mGRGfHOUg6WLr5BwzI/24gCytbFM8ZL5Ilj++57L8PW3lXjqBJsq8mfGLN6KWGwl h437P5W3hpPSEP8B/px8l4OrWSOa7cbQOSQjRQ0ijjLpqM1yMhdo= Received: (qmail 38195 invoked by alias); 13 Jan 2017 13:33:37 -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 38186 invoked by uid 89); 13 Jan 2017 13:33:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=secondly, heading, firstly, sk:lambda_ X-HELO: mail-yb0-f195.google.com Received: from mail-yb0-f195.google.com (HELO mail-yb0-f195.google.com) (209.85.213.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 13 Jan 2017 13:33:26 +0000 Received: by mail-yb0-f195.google.com with SMTP id w194so1784567ybe.1 for ; Fri, 13 Jan 2017 05:33:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:references:cc:from:message-id :date:user-agent:mime-version:in-reply-to; bh=x69f+6wR0i4rsqaciYD12duclJ1rwfBkCDydaXnd9SI=; b=HhJmO6uMVr2TqyDUwVb1wan23ZnAwyKOSeDf0QqFpFqddcTlNAreCMolx96P9FpEkv kfwsR/JoQ8AIogfuiFgqBT3l61XyacF6bwHAvgoac1u05lJdM4Ctoh6xI4ai5enUTZGn KOQttWzp4LjA3M7RFBWPVD9K5ltiSMUOtw9N42Os9jC5Kwt1ZkBcAPigWchtOR05mweo nS32p20eiDa/9gVPwyExv6P+/uEwL7isdfTqEoGEk2WWMz99xcA9UG9rGpV/bzs7zAIJ mrYpeIHTB9K3UqomHEqzvAYXk+1oO5YaKvcBcDlfpT57B8qrIJ1oVlzoEKfocz8ldlTA DUeQ== X-Gm-Message-State: AIkVDXI+ZP4TjpOldrj9ECvn5F8uL7AqM0FV7dwmnKxH7Lv73T/YG4+J9slgtWbMI0GSdQ== X-Received: by 10.37.163.66 with SMTP id d60mr12253784ybi.186.1484314404482; Fri, 13 Jan 2017 05:33:24 -0800 (PST) Received: from ?IPv6:2620:10d:c0a3:20fb:f6d0:5ac5:64cd:f102? ([2620:10d:c091:200::d:41ba]) by smtp.googlemail.com with ESMTPSA id m136sm5112791ywd.19.2017.01.13.05.33.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Jan 2017 05:33:23 -0800 (PST) Subject: Re: [C++ PATCH] c++/61636 generic lambdas and this capture To: Jason Merrill References: <54f8da9d-0c1a-0a9d-a5ef-2ecb6cdc28c4@acm.org> Cc: GCC Patches From: Nathan Sidwell Message-ID: <65f1a240-adf2-4b04-5839-3f378de40f1d@acm.org> Date: Fri, 13 Jan 2017 08:33:22 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <54f8da9d-0c1a-0a9d-a5ef-2ecb6cdc28c4@acm.org> On 12/21/2016 03:27 PM, Nathan Sidwell wrote: > This patch addresses bug 61636, which is an ICE during generic lambda > instantiation due to unexpected this capture. Here's an updated patch. Firstly I added the worker function to lambda.c, with a more descriptive name. Secondly, this implements a check that the overload set contains at least one non-static member fn. That's what Clang does, and discussion on the std list seems to be heading that way as the right approach. ok? nathan -- Nathan Sidwell 2016-12-21 Nathan Sidwell PR c++/61636 * cp-tree.h (maybe_generic_this_capture): Declare. * lambda.c (resolvable_dummy): New, broken out of ... (maybe_resolve_dummy): ... here. Call it. (maybe_generic_this_capture): New. * parser.c (cp_parser_postfix_expression): Speculatively capture this in generic lambda in unresolved member function call. * pt.c (tsubst_copy_and_build): Force hard error from failed member function lookup in generic lambda. PR c++/61636 * g++.dg/cpp1y/pr61636-1.C: New. * g++.dg/cpp1y/pr61636-2.C: New. * g++.dg/cpp1y/pr61636-3.C: New. Index: cp/cp-tree.h =================================================================== --- cp/cp-tree.h (revision 244382) +++ cp/cp-tree.h (working copy) @@ -6551,6 +6551,7 @@ extern bool is_capture_proxy (tree); extern bool is_normal_capture_proxy (tree); extern void register_capture_members (tree); extern tree lambda_expr_this_capture (tree, bool); +extern void maybe_generic_this_capture (tree, tree); extern tree maybe_resolve_dummy (tree, bool); extern tree current_nonlambda_function (void); extern tree nonlambda_method_basetype (void); Index: cp/lambda.c =================================================================== --- cp/lambda.c (revision 244382) +++ cp/lambda.c (working copy) @@ -793,16 +793,14 @@ lambda_expr_this_capture (tree lambda, b return result; } -/* We don't want to capture 'this' until we know we need it, i.e. after - overload resolution has chosen a non-static member function. At that - point we call this function to turn a dummy object into a use of the - 'this' capture. */ +/* Return the current LAMBDA_EXPR, if this is a resolvable dummy + object. NULL otherwise.. */ -tree -maybe_resolve_dummy (tree object, bool add_capture_p) +static tree +resolvable_dummy (tree object) { if (!is_dummy_object (object)) - return object; + return NULL_TREE; tree type = TYPE_MAIN_VARIANT (TREE_TYPE (object)); gcc_assert (!TYPE_PTR_P (type)); @@ -812,18 +810,55 @@ maybe_resolve_dummy (tree object, bool a && LAMBDA_TYPE_P (current_class_type) && lambda_function (current_class_type) && DERIVED_FROM_P (type, current_nonlambda_class_type ())) - { - /* In a lambda, need to go through 'this' capture. */ - tree lam = CLASSTYPE_LAMBDA_EXPR (current_class_type); - tree cap = lambda_expr_this_capture (lam, add_capture_p); - if (cap && cap != error_mark_node) + return CLASSTYPE_LAMBDA_EXPR (current_class_type); + + return NULL_TREE; +} + +/* We don't want to capture 'this' until we know we need it, i.e. after + overload resolution has chosen a non-static member function. At that + point we call this function to turn a dummy object into a use of the + 'this' capture. */ + +tree +maybe_resolve_dummy (tree object, bool add_capture_p) +{ + if (tree lam = resolvable_dummy (object)) + if (tree cap = lambda_expr_this_capture (lam, add_capture_p)) + if (cap != error_mark_node) object = build_x_indirect_ref (EXPR_LOCATION (object), cap, RO_NULL, tf_warning_or_error); - } return object; } +/* When parsing a generic lambda containing an argument-dependent + member function call we defer overload resolution to instantiation + time. But we have to know now whether to capture this or not. + Do that if FNS contains any non-static fns. + The std doesn't anticipate this case, but I expect this to be the + outcome of discussion. */ + +void +maybe_generic_this_capture (tree object, tree fns) +{ + if (tree lam = resolvable_dummy (object)) + if (!LAMBDA_EXPR_THIS_CAPTURE (lam)) + { + /* We've not yet captured, so look at the function set of + interest. */ + if (BASELINK_P (fns)) + fns = BASELINK_FUNCTIONS (fns); + for (; fns; fns = OVL_NEXT (fns)) + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (OVL_CURRENT (fns))) + { + /* Found a non-static member. Capture this. */ + lambda_expr_this_capture (lam, true); + break; + } + } +} + /* Returns the innermost non-lambda function. */ tree Index: cp/parser.c =================================================================== --- cp/parser.c (revision 244382) +++ cp/parser.c (working copy) @@ -6971,6 +6971,7 @@ cp_parser_postfix_expression (cp_parser || type_dependent_expression_p (fn) || any_type_dependent_arguments_p (args))) { + maybe_generic_this_capture (instance, fn); postfix_expression = build_nt_call_vec (postfix_expression, args); release_tree_vector (args); Index: cp/pt.c =================================================================== --- cp/pt.c (revision 244382) +++ cp/pt.c (working copy) @@ -17142,19 +17142,34 @@ tsubst_copy_and_build (tree t, if (unq != function) { - tree fn = unq; - if (INDIRECT_REF_P (fn)) - fn = TREE_OPERAND (fn, 0); - if (TREE_CODE (fn) == COMPONENT_REF) - fn = TREE_OPERAND (fn, 1); - if (is_overloaded_fn (fn)) - fn = get_first_fn (fn); - if (permerror (EXPR_LOC_OR_LOC (t, input_location), - "%qD was not declared in this scope, " - "and no declarations were found by " - "argument-dependent lookup at the point " - "of instantiation", function)) + /* In a lambda fn, we have to be careful to not + introduce new this captures. Legacy code can't + be using lambdas anyway, so it's ok to be + stricter. */ + bool in_lambda = (current_class_type + && LAMBDA_TYPE_P (current_class_type)); + char const *msg = "%qD was not declared in this scope, " + "and no declarations were found by " + "argument-dependent lookup at the point " + "of instantiation"; + + bool diag = true; + if (in_lambda) + error_at (EXPR_LOC_OR_LOC (t, input_location), + msg, function); + else + diag = permerror (EXPR_LOC_OR_LOC (t, input_location), + msg, function); + if (diag) { + tree fn = unq; + if (INDIRECT_REF_P (fn)) + fn = TREE_OPERAND (fn, 0); + if (TREE_CODE (fn) == COMPONENT_REF) + fn = TREE_OPERAND (fn, 1); + if (is_overloaded_fn (fn)) + fn = get_first_fn (fn); + if (!DECL_P (fn)) /* Can't say anything more. */; else if (DECL_CLASS_SCOPE_P (fn)) @@ -17177,7 +17192,13 @@ tsubst_copy_and_build (tree t, inform (DECL_SOURCE_LOCATION (fn), "%qD declared here, later in the " "translation unit", fn); + if (in_lambda) + { + release_tree_vector (call_args); + RETURN (error_mark_node); + } } + function = unq; } } Index: testsuite/g++.dg/cpp1y/pr61636-1.C =================================================================== --- testsuite/g++.dg/cpp1y/pr61636-1.C (revision 0) +++ testsuite/g++.dg/cpp1y/pr61636-1.C (working copy) @@ -0,0 +1,31 @@ +// PR c++/61636 +// { dg-do compile { target c++14 } } + +// ICE because we figure this capture too late. + +struct Base +{ + void Bar (int); +}; + +struct A : Base { + void b (); + void Foo (int); + using Base::Bar; + template void Baz (T); +}; + +void A::b() { + + auto lam = [&](auto asdf) { Foo (asdf); }; + + lam (0); + + auto lam1 = [&](auto asdf) { Bar (asdf); }; + + lam1 (0); + + auto lam2 = [&](auto asdf) { Baz (asdf); }; + + lam2 (0); +} Index: testsuite/g++.dg/cpp1y/pr61636-2.C =================================================================== --- testsuite/g++.dg/cpp1y/pr61636-2.C (revision 0) +++ testsuite/g++.dg/cpp1y/pr61636-2.C (working copy) @@ -0,0 +1,72 @@ +// PR c++/61636 +// { dg-do run { target c++14 } } + +// Check we don't capture this (too) unnecessarily + +struct A { + int b (); + void f (int) {} + static void f (double) {} + + static void g (int) {} + static void g (double) {} +}; + +struct O { + void x (int) {} + static void x (double) {} +}; + +namespace N { + void y (double) {} +} + +int Check (bool expect, unsigned size) +{ + return (expect ? sizeof (void *) : 1) != size; +} + +int A::b() { + int r = 0; + + // one of the functions is non-static + auto l0 = [&](auto z) { f (z); }; + r += Check (true, sizeof l0); + l0(0.0); // doesn't need this capture for A::f(double), but too late + l0 (0); // Needs this capture for A::f(int) + + // no fn is non-static. + auto l00 = [&](auto z) { g (z); }; + r += Check (false, sizeof l00); + l00(0.0); + l00 (0); + + // sizeof isn't an evaluation context, so no this capture + auto l1 = [&](auto z) { sizeof (f (z), 1); }; + r += Check (false, sizeof l1); + l1(0.0); l1 (0); + + auto l2 = [&](auto) { f (2.4); }; + auto l3 = [&](auto) { f (0); }; + l2(0); l3(0); l2(0.0); l3 (0.0); + r += Check (false, sizeof l2); + r += Check (true, sizeof l3); + + auto l4 = [&](auto) { O::x (2.4); }; + auto l5 = [&](auto) { N::y (2.4); }; + auto l6 = [&](auto) { }; + l4(0); l5(0); l6(0); + l4(0.0); l5(0.0); l6(0.0); + r += Check (false, sizeof l4); + r += Check (false, sizeof l5); + r += Check (false, sizeof l6); + + return r; +} + +int main () +{ + A a; + + return a.b () ? 1 : 0; +} Index: testsuite/g++.dg/cpp1y/pr61636-3.C =================================================================== --- testsuite/g++.dg/cpp1y/pr61636-3.C (revision 0) +++ testsuite/g++.dg/cpp1y/pr61636-3.C (working copy) @@ -0,0 +1,25 @@ +// PR c++/61636 +// { dg-do compile { target c++14 } } +// permissiveness doesn't make this permitted +// { dg-additional-options "-fpermissive" } + +// ICE because we attempt to use dependent Foo during error recovery +// and die with an unexpected this capture need. + +template struct Base +{ + void Foo (int); +}; + +template struct A : Base { + void b (); +}; + +template void A::b() { + + auto lam = [&](auto asdf) { Foo (asdf); }; // { dg-error "not declared" } + + lam (T(0)); +} + +template void A::b ();