From patchwork Tue Sep 14 04:31:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barrett Adair X-Patchwork-Id: 44969 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8333C3858039 for ; Tue, 14 Sep 2021 04:31:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8333C3858039 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1631593912; bh=6LuXgZNctNc2yqfqZ2fBGgrbDsYQaAp/jNlyJKwX4qQ=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=iT1kp8fxVIc1loiczf1Qz8yo3NB6xERO6W7r+OvaBfdOgwQkt4JmDXI3YLMLFfIqk zTWFRhnUNtQrddmS/F6BAdIfmGwOGfYdmhiJ6nR0HF7jW6N3gLQsx9q4sOP9yn4E8s mnX3b4VGbiJtnPd1115YMKRpVrtp3EjoMVqhRGFg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qk1-x734.google.com (mail-qk1-x734.google.com [IPv6:2607:f8b0:4864:20::734]) by sourceware.org (Postfix) with ESMTPS id 60DDC3858402 for ; Tue, 14 Sep 2021 04:31:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 60DDC3858402 Received: by mail-qk1-x734.google.com with SMTP id b64so13396179qkg.0 for ; Mon, 13 Sep 2021 21:31:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=65OW+N/b+ofXZQxUkv8bA5SQoKPSHWqpP4WMIy2aatw=; b=NLHu/kxem4EdiZoWvAM/akU5gE2VtEm2E0mV/ZgKO243PaKQlH8BUNCL5jVMwbPqpv ztO3vR6d1pvzQdmm7oqB0Rs52X3Fa9WhhaMxOQpRvvCZjEQctVuJJIDLUCjrA90XW4DF 0d45aLY/aABqYy1shQmg/6sM/xzf0fxongrYSbvGtXuWwpj25HUlTwgkBWbzpi7wEEqg y9be807KlSDwbF8FAGzZHfz69fN1tJ7wfc2DtLCASxXl2ZHulfrwvtAsgQiTkRXITrv1 I6YoBa9jji9u6a2zsedqlMcOgPUMwIES+X+JukqRCKSjh1fOsDil6Ss3DyXXgLZ2N95L hHhg== X-Gm-Message-State: AOAM530rioZ7cS6AIW9psPJvudOz1pZQX2yKR9XAot9CinGlkrDUwjuC P6CUBkLgh1uiITedlfyMMbNzx10LTuO/ix7uJJGGrn8KxOg= X-Google-Smtp-Source: ABdhPJyTUpWzvAQqRA2JnWDA2CFD9Y5gN740TVqIOIZylF3CvKjvZJmwU5vsrRaSpjkdn2tGdmXkCXz8aKFrCdlBcZI= X-Received: by 2002:a37:a13:: with SMTP id 19mr3011780qkk.285.1631593880663; Mon, 13 Sep 2021 21:31:20 -0700 (PDT) MIME-Version: 1.0 Date: Mon, 13 Sep 2021 23:31:10 -0500 Message-ID: Subject: [PATCH v3] c++: Fix cp_tree_equal for template value args using dependent sizeof/alignof/noexcept expressions To: gcc-patches@gcc.gnu.org X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: , X-Patchwork-Original-From: Barrett Adair via Gcc-patches From: Barrett Adair Reply-To: Barrett Adair Cc: jakub@redhat.com Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" I reworked the fix today based on feedback from Jason and Jakub (thank you), and the subject line is now outdated. I added another test for a closely related bug that's also fixed here (dependent-expr11.C -- this one would even fail without the second declaration). All the new tests in the patch succeed with the change (only two of them succeed with trunk). On my box, the bootstrap succeeds, the g++ test suite passes (matching today's posted results anyway), and the libstdc++ test suite is looking good but is still running after a long time. I'll leave the full "make check" running overnight. Some potentially controversial changes here: 1. Adding new bool member to cp_parser. I'd like to avoid this, any tips? 2. Relaxing an assert in tsubst_copy. This change feels correct to me, but maybe I'm missing something. 3. Pushing a function scope in PARM_DECL case in tsubst_copy_and_build to make process_outer_var_ref happy for trailing return types. I don't yet fully appreciate the consequences of these changes, so this needs some eyes. 4. Traversing each template arg's tree in any_template_arguments_need_structural_equality_p to identify dependent expressions in trailing return types. This could probably be done better. I check current_function_decl here as an optimization (since it's NULL in the only place that "needs" this), but that seems brittle. Also, the new find_dependent_parm_decl_r callback implementation may have the unintended consequence of forcing structural comparison on member function trailing return types that depend on class template parameters. I think I really only want to force structural comparison for "arg tree has a dependent parm decl and we're in a trailing return type" -- is there a better way to do this? Also note that I found another related bug which I have not yet solved: template struct foo { constexpr operator int() { return i; } }; void bar() { [](auto i) -> foo { return {}; }(foo<1>{}); } With the attached patch, failure occurs at invocation, while trunk fails to parse the return type. This seems like a step in the right direction, but we should consider whether such an incomplete fix introduces more issues than it solves (e.g. unfriendlier error messages, or perhaps something more sinister). Thanks, Barrett From 0470bdc5b2b4ddff2d2ee9db11a8c5895abda50f Mon Sep 17 00:00:00 2001 From: Barrett Adair Date: Fri, 20 Aug 2021 15:37:36 -0500 Subject: [PATCH] Fix trailing return type bugs --- gcc/cp/cp-tree.h | 2 +- gcc/cp/parser.c | 13 ++++- gcc/cp/parser.h | 3 + gcc/cp/pt.c | 58 +++++++++++++++++-- gcc/cp/semantics.c | 9 ++- gcc/testsuite/g++.dg/template/canon-type-15.C | 7 +++ gcc/testsuite/g++.dg/template/canon-type-16.C | 6 ++ gcc/testsuite/g++.dg/template/canon-type-17.C | 5 ++ gcc/testsuite/g++.dg/template/canon-type-18.C | 6 ++ .../g++.dg/template/dependent-expr11.C | 6 ++ .../g++.dg/template/dependent-name15.C | 18 ++++++ .../g++.dg/template/dependent-name16.C | 14 +++++ 12 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/canon-type-15.C create mode 100644 gcc/testsuite/g++.dg/template/canon-type-16.C create mode 100644 gcc/testsuite/g++.dg/template/canon-type-17.C create mode 100644 gcc/testsuite/g++.dg/template/canon-type-18.C create mode 100644 gcc/testsuite/g++.dg/template/dependent-expr11.C create mode 100644 gcc/testsuite/g++.dg/template/dependent-name15.C create mode 100644 gcc/testsuite/g++.dg/template/dependent-name16.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a82747ca627..b93455aebff 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7537,7 +7537,7 @@ extern tree process_outer_var_ref (tree, tsubst_flags_t, bool force_use = false extern cp_expr finish_id_expression (tree, tree, tree, cp_id_kind *, bool, bool, bool *, - bool, bool, bool, bool, + bool, bool, bool, bool, bool, const char **, location_t); extern tree finish_typeof (tree); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index e44c5c6b57c..4b95103eb2b 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -6011,7 +6011,8 @@ cp_parser_primary_expression (cp_parser *parser, parser->integral_constant_expression_p, parser->allow_non_integral_constant_expression_p, &parser->non_integral_constant_expression_p, - template_p, done, address_p, + template_p, parser->in_trailing_return_type_p, + done, address_p, template_arg_p, &error_msg, id_expression.get_location ())); @@ -11256,6 +11257,7 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr) /*allow_non_integral_constant_expression_p=*/false, /*non_integral_constant_expression_p=*/NULL, /*template_p=*/false, + /*trailing_return_type_p=*/false, /*done=*/true, /*address_p=*/false, /*template_arg_p=*/false, @@ -16192,6 +16194,7 @@ cp_parser_decltype_expr (cp_parser *parser, /*allow_non_integral_constant_expression_p=*/true, &non_integral_constant_expression_p, /*template_p=*/false, + parser->in_trailing_return_type_p, /*done=*/true, /*address_p=*/false, /*template_arg_p=*/false, @@ -24078,8 +24081,11 @@ cp_parser_template_type_arg (cp_parser *parser) static tree cp_parser_trailing_type_id (cp_parser *parser) { - return cp_parser_type_id_1 (parser, CP_PARSER_FLAGS_TYPENAME_OPTIONAL, + parser->in_trailing_return_type_p = true; + tree result = cp_parser_type_id_1 (parser, CP_PARSER_FLAGS_TYPENAME_OPTIONAL, false, true, NULL); + parser->in_trailing_return_type_p = false; + return result; } /* Parse a type-specifier-seq. @@ -44635,7 +44641,8 @@ cp_finish_omp_declare_variant (cp_parser *parser, cp_token *pragma_tok, = finish_id_expression (varid, variant, parser->scope, &idk, false, true, &parser->non_integral_constant_expression_p, - template_p, true, false, false, &error_msg, + template_p, parser->in_trailing_return_type_p, + true, false, false, &error_msg, varid.get_location ()); if (error_msg) cp_parser_error (parser, error_msg); diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h index 3669587cebd..a4f99e3f80c 100644 --- a/gcc/cp/parser.h +++ b/gcc/cp/parser.h @@ -419,6 +419,9 @@ struct GTY(()) cp_parser { member definition using a generic type, it is the sk_class scope. */ cp_binding_level* implicit_template_scope; + /* True if parsing a trailing return type.*/ + bool in_trailing_return_type_p; + /* True if parsing a result type in a compound requirement. This permits constrained-type-specifiers inside what would normally be a trailing return type. */ diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 224dd9ebd2b..20942c62a39 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -16593,10 +16593,9 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) if (DECL_NAME (t) == this_identifier && current_class_ptr) return current_class_ptr; - /* This can happen for a parameter name used later in a function - declaration (such as in a late-specified return type). Just - make a dummy decl, since it's only used for its type. */ - gcc_assert (cp_unevaluated_operand != 0); + gcc_assert (cp_unevaluated_operand != 0 + || TYPE_HAS_LATE_RETURN_TYPE (TREE_TYPE (DECL_CONTEXT (t)))); + r = tsubst_decl (t, args, complain); /* Give it the template pattern as its context; its true context hasn't been instantiated yet and this is good enough for @@ -19643,6 +19642,7 @@ tsubst_copy_and_build (tree t, /*allow_non_integral_constant_expression_p=*/(cxx_dialect >= cxx11), &non_integral_constant_expression_p, /*template_p=*/false, + /*in_trailing_return_type_p=*/false, /*done=*/true, /*address_p=*/false, /*template_arg_p=*/false, @@ -20859,6 +20859,25 @@ tsubst_copy_and_build (tree t, case PARM_DECL: { tree r = tsubst_copy (t, args, complain, in_decl); + tree context = r != error_mark_node + && tree_contains_struct[TREE_CODE (r)][TS_DECL_MINIMAL] + ? DECL_CONTEXT (r) : NULL_TREE; + + /* if we're instantiating a template in a trailing return type, + we need the function's parameters in scope */ + bool needs_context_push = context && TREE_TYPE (context) + && DECL_DECLARES_FUNCTION_P (context) + && TYPE_HAS_LATE_RETURN_TYPE (TREE_TYPE (context)); + + if (needs_context_push) + { + push_access_scope (context); + push_deferring_access_checks (dk_no_deferred); + if (LAMBDA_FUNCTION_P (context)) + start_lambda_scope (r); + ++function_depth; + } + /* ??? We're doing a subset of finish_id_expression here. */ if (tree wrap = maybe_get_tls_wrapper_call (r)) /* Replace an evaluated use of the thread_local variable with @@ -20871,6 +20890,16 @@ tsubst_copy_and_build (tree t, /* If the original type was a reference, we'll be wrapped in the appropriate INDIRECT_REF. */ r = convert_from_reference (r); + + if (needs_context_push) + { + --function_depth; + if (LAMBDA_FUNCTION_P (context)) + finish_lambda_scope (); + pop_deferring_access_checks (); + pop_access_scope (context); + } + RETURN (r); } @@ -27766,6 +27795,21 @@ dependent_template_arg_p (tree arg) return value_dependent_expression_p (arg); } +/* Identify any expressions dependent on a function + template's PARM_DECL */ +static tree +find_dependent_parm_decl_r (tree *tp, int *walk_subtrees, void*) +{ + tree t = *tp; + if (PACK_EXPANSION_P (t) || (TREE_CODE (t) == PARM_DECL + && TREE_CODE (TREE_TYPE (t)) == TEMPLATE_TYPE_PARM)) + { + *walk_subtrees = 0; + return t; + } + return NULL_TREE; +} + /* Returns true if ARGS (a collection of template arguments) contains any types that require structural equality testing. */ @@ -27810,6 +27854,12 @@ any_template_arguments_need_structural_equality_p (tree args) else if (!TYPE_P (arg) && TREE_TYPE (arg) && TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (arg))) return true; + /* if dependent instantiation inside trailing return type, + require structural comparison, at least until canonical + type is made to work for this case */ + else if (!current_function_decl && + cp_walk_tree_without_duplicates (&arg, find_dependent_parm_decl_r, NULL)) + return true; } } } diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 94e6b181d5d..d9cc52879b9 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -3932,6 +3932,7 @@ finish_id_expression_1 (tree id_expression, bool allow_non_integral_constant_expression_p, bool *non_integral_constant_expression_p, bool template_p, + bool trailing_return_type_p, bool done, bool address_p, bool template_arg_p, @@ -4007,7 +4008,7 @@ finish_id_expression_1 (tree id_expression, body, except inside an unevaluated context (i.e. decltype). */ if (TREE_CODE (decl) == PARM_DECL && DECL_CONTEXT (decl) == NULL_TREE - && !cp_unevaluated_operand) + && !cp_unevaluated_operand && !trailing_return_type_p) { *error_msg = G_("use of parameter outside function body"); return error_mark_node; @@ -4259,6 +4260,7 @@ finish_id_expression (tree id_expression, bool allow_non_integral_constant_expression_p, bool *non_integral_constant_expression_p, bool template_p, + bool trailing_return_type_p, bool done, bool address_p, bool template_arg_p, @@ -4270,7 +4272,8 @@ finish_id_expression (tree id_expression, integral_constant_expression_p, allow_non_integral_constant_expression_p, non_integral_constant_expression_p, - template_p, done, address_p, template_arg_p, + template_p, trailing_return_type_p, + done, address_p, template_arg_p, error_msg, location); return result.maybe_add_location_wrapper (); } @@ -5737,7 +5740,7 @@ omp_reduction_lookup (location_t loc, tree id, tree type, tree *baselinkp, if (decl == NULL_TREE) decl = error_mark_node; id = finish_id_expression (id, decl, NULL_TREE, &idk, false, true, - &nonint_cst_expression_p, false, true, false, + &nonint_cst_expression_p, false, false, true, false, false, &error_msg, loc); if (idk == CP_ID_KIND_UNQUALIFIED && identifier_p (id)) diff --git a/gcc/testsuite/g++.dg/template/canon-type-15.C b/gcc/testsuite/g++.dg/template/canon-type-15.C new file mode 100644 index 00000000000..b001b5c841d --- /dev/null +++ b/gcc/testsuite/g++.dg/template/canon-type-15.C @@ -0,0 +1,7 @@ +// { dg-do compile { target c++11 } } +template struct size_c{ static constexpr unsigned value = u; }; +namespace g { +template auto return_size(T t) -> size_c; +template auto return_size(T t) -> size_c; +} +static_assert(decltype(g::return_size('a'))::value == 1u, ""); diff --git a/gcc/testsuite/g++.dg/template/canon-type-16.C b/gcc/testsuite/g++.dg/template/canon-type-16.C new file mode 100644 index 00000000000..99361cbac30 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/canon-type-16.C @@ -0,0 +1,6 @@ +// { dg-do compile { target c++11 } } +template struct bool_c{ static constexpr bool value = u; }; +template auto noexcepty(T t) -> bool_c; +template auto noexcepty(T t) -> bool_c; +struct foo { void operator()() noexcept; }; +static_assert(decltype(noexcepty(foo{}))::value, ""); diff --git a/gcc/testsuite/g++.dg/template/canon-type-17.C b/gcc/testsuite/g++.dg/template/canon-type-17.C new file mode 100644 index 00000000000..0555c8d0a42 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/canon-type-17.C @@ -0,0 +1,5 @@ +// { dg-do compile { target c++11 } } +template struct size_c{ static constexpr unsigned value = u; }; +template auto return_size(T... t) -> size_c; +template auto return_size(T... t) -> size_c; +static_assert(decltype(return_size('a'))::value == 1u, ""); diff --git a/gcc/testsuite/g++.dg/template/canon-type-18.C b/gcc/testsuite/g++.dg/template/canon-type-18.C new file mode 100644 index 00000000000..2510181725c --- /dev/null +++ b/gcc/testsuite/g++.dg/template/canon-type-18.C @@ -0,0 +1,6 @@ +// { dg-do compile { target c++11 } } +// { dg-options "-Wno-pedantic" } +template struct size_c{ static constexpr unsigned value = u; }; +template auto get_align(T t) -> size_c; +template auto get_align(T t) -> size_c; +static_assert(decltype(get_align('a'))::value == 1u, ""); diff --git a/gcc/testsuite/g++.dg/template/dependent-expr11.C b/gcc/testsuite/g++.dg/template/dependent-expr11.C new file mode 100644 index 00000000000..d1fa63fc792 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/dependent-expr11.C @@ -0,0 +1,6 @@ +// { dg-do compile { target c++11 } } +template struct bool_c{ static constexpr bool value = u; }; +struct foo { constexpr bool operator()() const { return true; } }; +template auto bar(T t) -> bool_c; +template auto bar(T t) -> bool_c; +static_assert(decltype(bar(foo{}))::value, ""); diff --git a/gcc/testsuite/g++.dg/template/dependent-name15.C b/gcc/testsuite/g++.dg/template/dependent-name15.C new file mode 100644 index 00000000000..1c34bc704f9 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/dependent-name15.C @@ -0,0 +1,18 @@ +// { dg-do compile { target c++11 } } +template struct A { static void foo(){} }; +template <> struct A { using foo = int; }; + +template void f(T t1) { + A::foo(); +} + +template void g(T t2) { + /* if the comparing_specializations check breaks in cp_tree_equal + case PARM_DECL, the error will incorrectly report A */ + A::foo(); // { dg-error "dependent-name .A::foo" } +} + +void h() { + f(0); + g('0'); +} diff --git a/gcc/testsuite/g++.dg/template/dependent-name16.C b/gcc/testsuite/g++.dg/template/dependent-name16.C new file mode 100644 index 00000000000..ef8c4f23077 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/dependent-name16.C @@ -0,0 +1,14 @@ +// { dg-do compile { target c++11 } } +template struct A { static void foo(){} }; +template <> struct A { using foo = int; }; + +template auto f(T1 t1) -> decltype(A::foo()); + +/* if the comparing_specializations check breaks in cp_tree_equal +case PARM_DECL, the error will incorrectly report A */ +template auto g(T2 t2) -> decltype(A::foo()); // { dg-error "dependent-name .A::foo" } + +void h() { + f(0); + g('0'); // { dg-error "no matching function" } +} -- 2.30.2