On Mon, Mar 02, 2020 at 05:50:01PM -0500, Jason Merrill wrote: > On 2/29/20 2:32 PM, Marek Polacek wrote: > > The point of this patch is to fix the recurring problem of trees > > generated by convert_like while processing a template that break when > > substituting. For instance, when convert_like creates a CALL_EXPR > > while in a template, substituting such a call breaks in finish_call_expr > > because we have two 'this' arguments. Another problem is that we > > can create &TARGET_EXPR<> and then fail when substituting because we're > > taking the address of an rvalue. I've analyzed some of the already fixed > > PRs and also some of the currently open ones: > > > > In c++/93870 we create EnumWrapper<E>::operator E(&operator~(E)). > > In c++/87145 we create S::operator int (&{N}). > > In c++/92031 we create &TARGET_EXPR <0>. > > > > And so on. I'd like to fix it once and for all. I wanted something > > that fixes all the existing cases, removes the ugly check in > > convert_nontype_argument, and something suitable for stage4. I.e., > > I didn't implement any cleanups suggested in > > <https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00832.html> regarding > > the pattern in e.g. build_explicit_specifier. > > Hmm, it seems to me that addressing that pattern is an important part of > fixing this once and for all. > > > The gist of the problem is when convert_like_real creates a call for > > a ck_user or wraps a TARGET_EXPR in & in a template. So in these cases > > use IMPLICIT_CONV_EXPR. In a template we shouldn't need to perform the > > actual conversion, we only need it's result type. Is that something > > that convert_like_real shouldn't do? > > perform_direct_initialization_if_possible and > > perform_implicit_conversion_flags > > can also create an IMPLICIT_CONV_EXPR. > > This seems like a reasonable approach.
Great. > > Given the change above, build_converted_constant_expr can return an > > IMPLICIT_CONV_EXPR so call fold_non_dependent_expr rather than > > maybe_constant_value to deal with that. A problem with that is that now > > we may instantiate something twice in a row (?). > > Right, and we must not do that. Ack. > > Handling all of it in > > build_converted_constant_expr won't be that straightforward because we > > sometimes call cxx_constant_value to give errors, or use > > manifestly_const_eval > > which should be honored. > > Fair enough. The alternative is ensuring that it's OK to call > build_converted_constant_expr when processing_template_decl is true, and > that the result in that case is still template trees. I think that's what > you are doing. > > With this approach, we can change > > expr = instantiate_non_dependent_expr_sfinae (expr, complain); > /* Don't let convert_like_real create more template codes. */ > processing_template_decl_sentinel s; > expr = build_converted_constant_bool_expr (expr, complain); > expr = cxx_constant_value (expr); > > to > > expr = build_converted_constant_bool_expr (expr, complain); > expr = instantiate_non_dependent_expr_sfinae (expr, complain); > expr = cxx_constant_value (expr); > > Yes? Yes, that seems to work. Changed in this patch per the above. > > --- a/gcc/cp/call.c > > +++ b/gcc/cp/call.c > > @@ -7383,6 +7383,12 @@ convert_like_real (conversion *convs, tree expr, > > tree fn, int argnum, > > { > > struct z_candidate *cand = convs->cand; > > + /* Creating &TARGET_EXPR<> in a template breaks when substituting, > > + and creating a CALL_EXPR in a template breaks in finish_call_expr > > + so use an IMPLICIT_CONV_EXPR for this conversion. */ > > + if (processing_template_decl) > > + return build1 (IMPLICIT_CONV_EXPR, totype, expr); > > + > > if (cand == NULL) > > /* We chose the surrogate function from add_conv_candidate, now we > > actually need to build the conversion. */ > > @@ -7760,6 +7766,12 @@ convert_like_real (conversion *convs, tree expr, > > tree fn, int argnum, > > expr = convert_bitfield_to_declared_type (expr); > > expr = fold_convert (type, expr); > > } > > + > > + /* Creating &TARGET_EXPR<> in a template would break when > > + tsubsting the expression, so use an IMPLICIT_CONV_EXPR > > + instead. */ > > + if (processing_template_decl) > > + return build1 (IMPLICIT_CONV_EXPR, totype, expr); > > expr = build_target_expr_with_type (expr, type, complain); > > } > > Don't you need the same thing for ck_list, ck_aggr, ck_base? Is there a > reason not to do this in one place for anything other than ck_identity, at > least if a class type is involved? I didn't think I needed it because it didn't look like we might create either &TARGET_EXPR or CALL_EXPR there. But maybe it'd be safer to do it even for ck_list, ck_aggr, ck_base, so changed. But the ck_ref_bind hunk had to stay: we can create a TARGET_EXPR even when there's no class involved in the conversion, e.g. converting and integer to a reference type, when a temporary is needed. > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > > index e3f4b435a49..d04b042f880 100644 > > --- a/gcc/cp/decl.c > > +++ b/gcc/cp/decl.c > > @@ -10281,8 +10281,8 @@ compute_array_index_type_loc (location_t name_loc, > > tree name, tree size, > > /* Pedantically a constant expression is required here and so > > __builtin_is_constant_evaluated () should fold to true if it > > is successfully folded into a constant. */ > > - size = maybe_constant_value (size, NULL_TREE, > > - /*manifestly_const_eval=*/true); > > + size = fold_non_dependent_expr (size, complain, > > + /*manifestly_const_eval=*/true); > > Here's that broken double-instantiation. Does it work if you also remove > the instantiate_non_dependent_expr before the call to > build_converted_constant_expr? Yes! Since this code here is guarded with !type_dependent_expression_p, I was wondering what would happen when we have a value-dependent expression; added new test for that. fold_non_dependent_expr_template will see that the expr is not is_nondependent_constant_expression, so it just returns it, it's not TREE_CONSTANT, so we use origsize. Meanwhile, PR94068 was opened, another instance of this problem. New test added. Bootstrapped/regtested on x86_64-linux, ok for trunk? -- >8 -- The point of this patch is to fix the recurring problem of trees generated by convert_like while processing a template that break when substituting. For instance, when convert_like creates a CALL_EXPR while in a template, substituting such a call breaks in finish_call_expr because we have two 'this' arguments. Another problem is that we can create &TARGET_EXPR<> and then fail when substituting because we're taking the address of an rvalue. I've analyzed some of the already fixed PRs and also some of the currently open ones: In c++/93870 we create EnumWrapper<E>::operator E(&operator~(E)). In c++/87145 we create S::operator int (&{N}). In c++/92031 we create &TARGET_EXPR <0>. The gist of the problem is when convert_like_real creates a call for a ck_user or wraps a TARGET_EXPR in & in a template. So in these cases use IMPLICIT_CONV_EXPR. In a template we shouldn't need to perform the actual conversion, we only need it's result type. perform_direct_initialization_if_possible and perform_implicit_conversion_flags can also create an IMPLICIT_CONV_EXPR. Given the change above, build_converted_constant_expr can return an IMPLICIT_CONV_EXPR so call fold_non_dependent_expr rather than maybe_constant_value to deal with that. To avoid the problem of instantiating something twice in a row I'm removing a call to instantiate_non_dependent_expr_sfinae in compute_array_index_type_loc. And the build_converted_constant_expr pattern can now be simplified. 2020-03-09 Marek Polacek <pola...@redhat.com> PR c++/92031 - bogus taking address of rvalue error. PR c++/91465 - ICE with template codes in check_narrowing. PR c++/93870 - wrong error when converting template non-type arg. PR c++/94068 - ICE with template codes in check_narrowing. * call.c (convert_like_real): Return IMPLICIT_CONV_EXPR in a template when not ck_identity and we're dealing with a class. (convert_like_real) <case ck_ref_bind>: Return IMPLICIT_CONV_EXPR in a template if we need a temporary. * decl.c (compute_array_index_type_loc): Remove instantiate_non_dependent_expr_sfinae call. Call fold_non_dependent_expr instead of maybe_constant_value. (build_explicit_specifier): Don't instantiate or create a sentinel before converting the expression. * except.c (build_noexcept_spec): Likewise. * pt.c (convert_nontype_argument): Don't build IMPLICIT_CONV_EXPR. Set IMPLICIT_CONV_EXPR_NONTYPE_ARG if that's what build_converted_constant_expr returned. * typeck2.c (check_narrowing): Call fold_non_dependent_expr instead of maybe_constant_value. * g++.dg/cpp0x/conv-tmpl2.C: New test. * g++.dg/cpp0x/conv-tmpl3.C: New test. * g++.dg/cpp0x/conv-tmpl4.C: New test. * g++.dg/cpp0x/conv-tmpl5.C: New test. * g++.dg/cpp0x/conv-tmpl6.C: New test. * g++.dg/cpp1z/conv-tmpl1.C: New test. --- gcc/cp/call.c | 18 ++++++++++++++ gcc/cp/decl.c | 9 +++---- gcc/cp/except.c | 4 +-- gcc/cp/pt.c | 25 ++++--------------- gcc/cp/typeck2.c | 6 ++++- gcc/testsuite/g++.dg/cpp0x/conv-tmpl2.C | 21 ++++++++++++++++ gcc/testsuite/g++.dg/cpp0x/conv-tmpl3.C | 16 ++++++++++++ gcc/testsuite/g++.dg/cpp0x/conv-tmpl4.C | 33 +++++++++++++++++++++++++ gcc/testsuite/g++.dg/cpp0x/conv-tmpl5.C | 13 ++++++++++ gcc/testsuite/g++.dg/cpp0x/conv-tmpl6.C | 16 ++++++++++++ gcc/testsuite/g++.dg/cpp1z/conv-tmpl1.C | 10 ++++++++ 11 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/conv-tmpl2.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/conv-tmpl3.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/conv-tmpl4.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/conv-tmpl5.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/conv-tmpl6.C create mode 100644 gcc/testsuite/g++.dg/cpp1z/conv-tmpl1.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index c0340d96f3c..5767a8b8c4e 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -7377,6 +7377,16 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, if (issue_conversion_warnings && (complain & tf_warning)) conversion_null_warnings (totype, expr, fn, argnum); + /* Creating &TARGET_EXPR<> in a template breaks when substituting, + and creating a CALL_EXPR in a template breaks in finish_call_expr + so use an IMPLICIT_CONV_EXPR for this conversion. We would have + created such codes e.g. when calling a user-defined conversion + function. */ + if (processing_template_decl + && convs->kind != ck_identity + && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))) + return build1 (IMPLICIT_CONV_EXPR, totype, expr); + switch (convs->kind) { case ck_user: @@ -7763,6 +7773,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, expr = convert_bitfield_to_declared_type (expr); expr = fold_convert (type, expr); } + + /* Creating &TARGET_EXPR<> in a template would break when + tsubsting the expression, so use an IMPLICIT_CONV_EXPR + instead. This can happen even when there's no class + involved, e.g., when converting an integer to a reference + type. */ + if (processing_template_decl) + return build1 (IMPLICIT_CONV_EXPR, totype, expr); expr = build_target_expr_with_type (expr, type, complain); } diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index e3f4b435a49..bb242743074 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -10276,13 +10276,12 @@ compute_array_index_type_loc (location_t name_loc, tree name, tree size, NOP_EXPR with TREE_SIDE_EFFECTS; don't fold in that case. */; else { - size = instantiate_non_dependent_expr_sfinae (size, complain); size = build_converted_constant_expr (size_type_node, size, complain); /* Pedantically a constant expression is required here and so __builtin_is_constant_evaluated () should fold to true if it is successfully folded into a constant. */ - size = maybe_constant_value (size, NULL_TREE, - /*manifestly_const_eval=*/true); + size = fold_non_dependent_expr (size, complain, + /*manifestly_const_eval=*/true); if (!TREE_CONSTANT (size)) size = origsize; @@ -17607,10 +17606,8 @@ build_explicit_specifier (tree expr, tsubst_flags_t complain) /* Wait for instantiation, tsubst_function_decl will handle it. */ return expr; - expr = instantiate_non_dependent_expr_sfinae (expr, complain); - /* Don't let convert_like_real create more template codes. */ - processing_template_decl_sentinel s; expr = build_converted_constant_bool_expr (expr, complain); + expr = instantiate_non_dependent_expr_sfinae (expr, complain); expr = cxx_constant_value (expr); return expr; } diff --git a/gcc/cp/except.c b/gcc/cp/except.c index 788b96de243..262ba5d309c 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1299,10 +1299,8 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain) if (TREE_CODE (expr) != DEFERRED_NOEXCEPT && !value_dependent_expression_p (expr)) { - expr = instantiate_non_dependent_expr_sfinae (expr, complain); - /* Don't let convert_like_real create more template codes. */ - processing_template_decl_sentinel s; expr = build_converted_constant_bool_expr (expr, complain); + expr = instantiate_non_dependent_expr_sfinae (expr, complain); expr = cxx_constant_value (expr); } if (TREE_CODE (expr) == INTEGER_CST) diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 1c721b31176..49ee3920049 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -7068,26 +7068,6 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain) else if (INTEGRAL_OR_ENUMERATION_TYPE_P (type) || cxx_dialect >= cxx17) { - /* Calling build_converted_constant_expr might create a call to - a conversion function with a value-dependent argument, which - could invoke taking the address of a temporary representing - the result of the conversion. */ - if (!same_type_ignoring_top_level_qualifiers_p (type, expr_type) - && ((COMPOUND_LITERAL_P (expr) - && CONSTRUCTOR_IS_DEPENDENT (expr) - && MAYBE_CLASS_TYPE_P (expr_type) - && TYPE_HAS_CONVERSION (expr_type)) - /* Similarly, converting e.g. an integer to a class - involves a constructor call. convert_like would - create a TARGET_EXPR, but in a template we can't - use AGGR_INIT_EXPR, and the TARGET_EXPR would lead - to a bogus error. */ - || (val_dep_p && MAYBE_CLASS_TYPE_P (type)))) - { - expr = build1 (IMPLICIT_CONV_EXPR, type, expr); - IMPLICIT_CONV_EXPR_NONTYPE_ARG (expr) = true; - return expr; - } /* C++17: A template-argument for a non-type template-parameter shall be a converted constant expression (8.20) of the type of the template-parameter. */ @@ -7096,6 +7076,11 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain) /* Make sure we return NULL_TREE only if we have really issued an error, as described above. */ return (complain & tf_error) ? NULL_TREE : error_mark_node; + else if (TREE_CODE (expr) == IMPLICIT_CONV_EXPR) + { + IMPLICIT_CONV_EXPR_NONTYPE_ARG (expr) = true; + return expr; + } expr = maybe_constant_value (expr, NULL_TREE, /*manifestly_const_eval=*/true); expr = convert_from_reference (expr); diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c index 48920894b3b..bff4ddbcf81 100644 --- a/gcc/cp/typeck2.c +++ b/gcc/cp/typeck2.c @@ -981,7 +981,11 @@ check_narrowing (tree type, tree init, tsubst_flags_t complain, return ok; } - init = maybe_constant_value (init); + /* Even non-dependent expressions can still have template + codes like CAST_EXPR, so use *_non_dependent_expr to cope. */ + init = fold_non_dependent_expr (init, complain); + if (init == error_mark_node) + return ok; /* If we were asked to only check constants, return early. */ if (const_only && !TREE_CONSTANT (init)) diff --git a/gcc/testsuite/g++.dg/cpp0x/conv-tmpl2.C b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl2.C new file mode 100644 index 00000000000..8a505769c3c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl2.C @@ -0,0 +1,21 @@ +// PR c++/92031 - bogus taking address of rvalue error. +// { dg-do compile { target c++11 } } + +struct x { const int& l; }; + +void a(const x&) {} + +template<class E> +void f() { + a(x { 0 }); +} + +void g() { + a(x { 0 }); +} + +void +test () +{ + f<int>(); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/conv-tmpl3.C b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl3.C new file mode 100644 index 00000000000..e2021aa13e1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl3.C @@ -0,0 +1,16 @@ +// PR c++/91465 - ICE with template codes in check_narrowing. +// { dg-do compile { target c++11 } } + +enum class D { X }; +enum class S { Z }; + +D foo(S) { return D{}; } +D foo(double) { return D{}; } + +template <typename> +struct Bar { + D baz(S s) + { + return D{foo(s)}; + } +}; diff --git a/gcc/testsuite/g++.dg/cpp0x/conv-tmpl4.C b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl4.C new file mode 100644 index 00000000000..966a2e1ac9e --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl4.C @@ -0,0 +1,33 @@ +// PR c++/93870 - wrong error when converting template non-type arg. +// { dg-do compile { target c++11 } } + +template <typename ENUM> struct EnumWrapper +{ + ENUM value; + + constexpr operator ENUM() const + { + return value; + } +}; + +enum E : int { V }; + +constexpr EnumWrapper<E> operator ~(E a) +{ + return {E(~int(a))}; +} + +template <E X> struct R +{ + static void Func(); +}; + +template <E X> struct S : R<~X> +{ +}; + +void Test() +{ + S<E::V>::Func(); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/conv-tmpl5.C b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl5.C new file mode 100644 index 00000000000..c83e6d83ed9 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl5.C @@ -0,0 +1,13 @@ +// PR c++/94068 - ICE with template codes in check_narrowing. +// { dg-do compile { target c++11 } } + +enum class A { A1, A2 }; +A foo (); +long foo (int); + +template <typename> +void +bar () +{ + const auto c{foo ()}; +} diff --git a/gcc/testsuite/g++.dg/cpp0x/conv-tmpl6.C b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl6.C new file mode 100644 index 00000000000..2df3a6cc129 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/conv-tmpl6.C @@ -0,0 +1,16 @@ +// { dg-do compile { target c++11 } } + +struct A +{ + constexpr A(int) { } + constexpr operator int() const { return 1; }; +}; + +template <class T, int N> +struct B +{ + static constexpr A a = A(N); + int ar[a]; +}; + +B<int, 10> b; diff --git a/gcc/testsuite/g++.dg/cpp1z/conv-tmpl1.C b/gcc/testsuite/g++.dg/cpp1z/conv-tmpl1.C new file mode 100644 index 00000000000..5b1205349d0 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/conv-tmpl1.C @@ -0,0 +1,10 @@ +// PR c++/91465 - ICE with template codes in check_narrowing. +// { dg-do compile { target c++17 } } + +enum class E { Z }; + +template <typename F> +void foo(F) +{ + E{char(0)}; +} base-commit: 81fa6d7321dd9b645d86de4a8a6967c603f176e3 -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA