* [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] @ 2020-06-23 2:09 Marek Polacek 2020-06-29 15:01 ` Marek Polacek 2020-07-03 21:24 ` Jason Merrill 0 siblings, 2 replies; 8+ messages in thread From: Marek Polacek @ 2020-06-23 2:09 UTC (permalink / raw) To: Jason Merrill, GCC Patches convert_like issues errors about bad_p conversions at the beginning of the function, but in the ck_ref_bind case, it only issues them after we've called convert_like on the next conversion. This doesn't work as expected since r10-7096 because when we see a conversion from/to class type in a template, we return early, thereby missing the error, and a bad_p conversion goes by undetected. That made the attached test to compile even though it should not. I had thought that I could just move the ck_ref_bind/bad_p errors above to the rest of them, but that regressed diagnostics because expr then wasn't converted yet by the nested convert_like_real call. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 10? gcc/cp/ChangeLog: PR c++/95789 * call.c (convert_like_real): Do the normal processing for ck_ref_bind conversion that are bad_p. gcc/testsuite/ChangeLog: PR c++/95789 * g++.dg/conversion/ref4.C: New test. --- gcc/cp/call.c | 9 ++++++++- gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 2b39a3700fc..7b16895d5db 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -7402,7 +7402,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, function. */ if (processing_template_decl && convs->kind != ck_identity - && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))) + && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr))) + /* Do the normal processing to give the bad_p errors in ck_ref_bind + to avoid losing the fact that this conversion was bad. Since we + are going to return error_mark_node, we don't care about trees + breaking in templates. */ + && !(convs->kind == ck_ref_bind + && convs->bad_p + && !next_conversion (convs)->bad_p)) { expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C new file mode 100644 index 00000000000..464a4cf6c0f --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/ref4.C @@ -0,0 +1,22 @@ +// PR c++/95789 +// { dg-do compile { target c++11 } } + +struct B { + int n; +}; + +template <typename T> +struct A { + B& get() const { return f; } // { dg-error "binding reference" } + + B f; +}; + +int main() { + A<int> a; + a.f = {}; + + a.get().n = 10; + if (a.f.n != 0) + __builtin_abort(); +} base-commit: 0164e59835de81d758fd4c56248ad7a46435fbfa -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-06-23 2:09 [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] Marek Polacek @ 2020-06-29 15:01 ` Marek Polacek 2020-07-03 21:24 ` Jason Merrill 1 sibling, 0 replies; 8+ messages in thread From: Marek Polacek @ 2020-06-29 15:01 UTC (permalink / raw) To: Jason Merrill, GCC Patches Ping. On Mon, Jun 22, 2020 at 10:09:27PM -0400, Marek Polacek via Gcc-patches wrote: > convert_like issues errors about bad_p conversions at the beginning > of the function, but in the ck_ref_bind case, it only issues them > after we've called convert_like on the next conversion. > > This doesn't work as expected since r10-7096 because when we see > a conversion from/to class type in a template, we return early, thereby > missing the error, and a bad_p conversion goes by undetected. That > made the attached test to compile even though it should not. > > I had thought that I could just move the ck_ref_bind/bad_p errors > above to the rest of them, but that regressed diagnostics because > expr then wasn't converted yet by the nested convert_like_real call. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 10? > > gcc/cp/ChangeLog: > > PR c++/95789 > * call.c (convert_like_real): Do the normal processing for > ck_ref_bind conversion that are bad_p. > > gcc/testsuite/ChangeLog: > > PR c++/95789 > * g++.dg/conversion/ref4.C: New test. > --- > gcc/cp/call.c | 9 ++++++++- > gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++++++++++++ > 2 files changed, 30 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 2b39a3700fc..7b16895d5db 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -7402,7 +7402,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > function. */ > if (processing_template_decl > && convs->kind != ck_identity > - && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))) > + && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr))) > + /* Do the normal processing to give the bad_p errors in ck_ref_bind > + to avoid losing the fact that this conversion was bad. Since we > + are going to return error_mark_node, we don't care about trees > + breaking in templates. */ > + && !(convs->kind == ck_ref_bind > + && convs->bad_p > + && !next_conversion (convs)->bad_p)) > { > expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); > return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); > diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C > new file mode 100644 > index 00000000000..464a4cf6c0f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref4.C > @@ -0,0 +1,22 @@ > +// PR c++/95789 > +// { dg-do compile { target c++11 } } > + > +struct B { > + int n; > +}; > + > +template <typename T> > +struct A { > + B& get() const { return f; } // { dg-error "binding reference" } > + > + B f; > +}; > + > +int main() { > + A<int> a; > + a.f = {}; > + > + a.get().n = 10; > + if (a.f.n != 0) > + __builtin_abort(); > +} > > base-commit: 0164e59835de81d758fd4c56248ad7a46435fbfa > -- > Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA > Marek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-06-23 2:09 [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] Marek Polacek 2020-06-29 15:01 ` Marek Polacek @ 2020-07-03 21:24 ` Jason Merrill 2020-07-08 20:35 ` [PATCH v2] " Marek Polacek 1 sibling, 1 reply; 8+ messages in thread From: Jason Merrill @ 2020-07-03 21:24 UTC (permalink / raw) To: Marek Polacek, GCC Patches On 6/22/20 10:09 PM, Marek Polacek wrote: > convert_like issues errors about bad_p conversions at the beginning > of the function, but in the ck_ref_bind case, it only issues them > after we've called convert_like on the next conversion. > > This doesn't work as expected since r10-7096 because when we see > a conversion from/to class type in a template, we return early, thereby > missing the error, and a bad_p conversion goes by undetected. That > made the attached test to compile even though it should not. Hmm, why isn't there an error at instantiation time? Though giving an error at template parsing time is definitely preferable. > I had thought that I could just move the ck_ref_bind/bad_p errors > above to the rest of them, but that regressed diagnostics because > expr then wasn't converted yet by the nested convert_like_real call. Yeah, the early section is really just for scalar conversions. It would probably be good to do normal processing for all other bad conversions and only afterward build the IMPLICIT_CONV_EXPR if we aren't returning error_mark_node. Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-07-03 21:24 ` Jason Merrill @ 2020-07-08 20:35 ` Marek Polacek 2020-07-11 14:32 ` Jason Merrill 0 siblings, 1 reply; 8+ messages in thread From: Marek Polacek @ 2020-07-08 20:35 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Fri, Jul 03, 2020 at 05:24:34PM -0400, Jason Merrill via Gcc-patches wrote: > On 6/22/20 10:09 PM, Marek Polacek wrote: > > convert_like issues errors about bad_p conversions at the beginning > > of the function, but in the ck_ref_bind case, it only issues them > > after we've called convert_like on the next conversion. > > > > This doesn't work as expected since r10-7096 because when we see > > a conversion from/to class type in a template, we return early, thereby > > missing the error, and a bad_p conversion goes by undetected. That > > made the attached test to compile even though it should not. > > Hmm, why isn't there an error at instantiation time? We threw away the result: we're called from 12213 if (complain & tf_error) 12214 { 12215 if (conv) 12216 convert_like (conv, expr, complain); ... 12228 return error_mark_node; and convert_like never saw converting this->f to B& again when instantiating. > Though giving an error at template parsing time is definitely preferable. Yup. > > I had thought that I could just move the ck_ref_bind/bad_p errors > > above to the rest of them, but that regressed diagnostics because > > expr then wasn't converted yet by the nested convert_like_real call. > > Yeah, the early section is really just for scalar conversions. > > It would probably be good to do normal processing for all other bad > conversions and only afterward build the IMPLICIT_CONV_EXPR if we aren't > returning error_mark_node. Ok, so that if we add more bad_p errors, we won't run into this again. Unfortunately it's a bit ugly. I could introduce a RETURN macro to use RETURN (expr); instead of what I have now, but it wouldn't be simply "conv_expr ? conv_expr : expr", because if we have error_mark_node, we want to return that, not conv_expr. Does that seem worth it? (I wish I could at least use the op0 ?: op1 GNU extension.) I've also added another test. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- convert_like issues errors about bad_p conversions at the beginning of the function, but in the ck_ref_bind case, it only issues them after we've called convert_like on the next conversion. This doesn't work as expected since r10-7096 because when we see a conversion from/to class type in a template, we return early, thereby missing the error, and a bad_p conversion goes by undetected. That made the attached test to compile even though it should not. I had thought that I could just move the ck_ref_bind/bad_p errors above to the rest of them, but that regressed diagnostics because expr then wasn't converted yet by the nested convert_like_real call. So, for bad_p conversions, do the normal processing, but still return the IMPLICIT_CONV_EXPR to avoid introducing trees that the template processing can't handle well. gcc/cp/ChangeLog: PR c++/95789 * call.c (convert_like_real): Do the normal processing for conversion that are bad_p. Return the IMPLICIT_CONV_EXPR instead of EXPR if we're processing a bad_p conversion in a template. gcc/testsuite/ChangeLog: PR c++/95789 * g++.dg/conversion/ref4.C: New test. * g++.dg/conversion/ref5.C: New test. --- gcc/cp/call.c | 47 +++++++++++++++----------- gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++ gcc/testsuite/g++.dg/conversion/ref5.C | 14 ++++++++ 3 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 5341a572980..65565fc90a8 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -7400,12 +7400,19 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, so use an IMPLICIT_CONV_EXPR for this conversion. We would have created such codes e.g. when calling a user-defined conversion function. */ + tree conv_expr = NULL_TREE; if (processing_template_decl && convs->kind != ck_identity && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))) { - expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); - return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); + conv_expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); + if (convs->kind != ck_ref_bind) + conv_expr = convert_from_reference (conv_expr); + if (!convs->bad_p) + return conv_expr; + /* Do the normal processing to give the bad_p errors. But we still + need to return the IMPLICIT_CONV_EXPR, unless we're returning + error_mark_node. */ } switch (convs->kind) @@ -7465,7 +7472,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, TARGET_EXPR_LIST_INIT_P (expr) = true; TARGET_EXPR_DIRECT_INIT_P (expr) = direct; } - return expr; + return conv_expr ? conv_expr : expr; } /* We don't know here whether EXPR is being used as an lvalue or @@ -7488,7 +7495,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, TARGET_EXPR_LIST_INIT_P (expr) = true; } - return expr; + return conv_expr ? conv_expr : expr; } case ck_identity: if (BRACE_ENCLOSED_INITIALIZER_P (expr)) @@ -7513,7 +7520,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, continue to warn about uses of EXPR as an integer, rather than as a pointer. */ expr = build_int_cst (totype, 0); - return expr; + return conv_expr ? conv_expr : expr; case ck_ambig: /* We leave bad_p off ck_ambig because overload resolution considers it valid, it just fails when we try to perform it. So we need to @@ -7585,7 +7592,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, field = next_initializable_field (DECL_CHAIN (field)); CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len)); tree new_ctor = build_constructor (totype, vec); - return get_target_expr_sfinae (new_ctor, complain); + return (conv_expr ? conv_expr + : get_target_expr_sfinae (new_ctor, complain)); } case ck_aggr: @@ -7598,14 +7606,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, imag = perform_implicit_conversion (TREE_TYPE (totype), imag, complain); expr = build2 (COMPLEX_EXPR, totype, real, imag); - return expr; + return conv_expr ? conv_expr : expr; } expr = reshape_init (totype, expr, complain); expr = get_target_expr_sfinae (digest_init (totype, expr, complain), complain); if (expr != error_mark_node) TARGET_EXPR_LIST_INIT_P (expr) = true; - return expr; + return conv_expr ? conv_expr : expr; default: break; @@ -7634,19 +7642,19 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, } if (! MAYBE_CLASS_TYPE_P (totype)) - return expr; + return conv_expr ? conv_expr : expr; /* Don't introduce copies when passing arguments along to the inherited constructor. */ if (current_function_decl && flag_new_inheriting_ctors && DECL_INHERITED_CTOR (current_function_decl)) - return expr; + return conv_expr ? conv_expr : expr; if (TREE_CODE (expr) == TARGET_EXPR && TARGET_EXPR_LIST_INIT_P (expr)) /* Copy-list-initialization doesn't actually involve a copy. */ - return expr; + return conv_expr ? conv_expr : expr; /* Fall through. */ case ck_base: @@ -7657,7 +7665,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, /* Build an expression for `*((base*) &expr)'. */ expr = convert_to_base (expr, totype, !c_cast_p, /*nonnull=*/true, complain); - return expr; + return conv_expr ? conv_expr : expr; } /* Copy-initialization where the cv-unqualified version of the source @@ -7686,7 +7694,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum); } - return build_cplus_new (totype, expr, complain); + return conv_expr ? conv_expr : build_cplus_new (totype, expr, complain); case ck_ref_bind: { @@ -7821,16 +7829,16 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, expr = cp_convert (build_pointer_type (TREE_TYPE (ref_type)), expr, complain); /* Convert the pointer to the desired reference type. */ - return build_nop (ref_type, expr); + return conv_expr ? conv_expr : build_nop (ref_type, expr); } case ck_lvalue: - return decay_conversion (expr, complain); + return conv_expr ? conv_expr : decay_conversion (expr, complain); case ck_fnptr: /* ??? Should the address of a transaction-safe pointer point to the TM clone, and this conversion look up the primary function? */ - return build_nop (totype, expr); + return conv_expr ? conv_expr : build_nop (totype, expr); case ck_qual: /* Warn about deprecated conversion if appropriate. */ @@ -7845,11 +7853,12 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, if (convs->base_p) expr = convert_to_base (expr, totype, !c_cast_p, /*nonnull=*/false, complain); - return build_nop (totype, expr); + return conv_expr ? conv_expr : build_nop (totype, expr); case ck_pmem: - return convert_ptrmem (totype, expr, /*allow_inverse_p=*/false, + expr = convert_ptrmem (totype, expr, /*allow_inverse_p=*/false, c_cast_p, complain); + return conv_expr ? conv_expr : expr; default: break; @@ -7866,7 +7875,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, else expr = cp_convert (totype, expr, complain); - return expr; + return conv_expr ? conv_expr : expr; } /* ARG is being passed to a varargs function. Perform any conversions diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C new file mode 100644 index 00000000000..464a4cf6c0f --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/ref4.C @@ -0,0 +1,22 @@ +// PR c++/95789 +// { dg-do compile { target c++11 } } + +struct B { + int n; +}; + +template <typename T> +struct A { + B& get() const { return f; } // { dg-error "binding reference" } + + B f; +}; + +int main() { + A<int> a; + a.f = {}; + + a.get().n = 10; + if (a.f.n != 0) + __builtin_abort(); +} diff --git a/gcc/testsuite/g++.dg/conversion/ref5.C b/gcc/testsuite/g++.dg/conversion/ref5.C new file mode 100644 index 00000000000..0042acd0670 --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/ref5.C @@ -0,0 +1,14 @@ +// PR c++/96104 + +template <typename T> void fn(T &); +class E {}; +struct F { + template <typename T> void mfn(T t) { t, fn(E()); } // { dg-error "cannot bind non-const lvalue reference" } +}; +int +main() +{ + E e; + F f; + f.mfn(e); +} base-commit: 760df6d296b8fc59796f42dca5eb14012fbfa28b -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-07-08 20:35 ` [PATCH v2] " Marek Polacek @ 2020-07-11 14:32 ` Jason Merrill 2020-07-13 20:48 ` [PATCH v3] " Marek Polacek 0 siblings, 1 reply; 8+ messages in thread From: Jason Merrill @ 2020-07-11 14:32 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 7/8/20 4:35 PM, Marek Polacek wrote: > On Fri, Jul 03, 2020 at 05:24:34PM -0400, Jason Merrill via Gcc-patches wrote: >> On 6/22/20 10:09 PM, Marek Polacek wrote: >>> convert_like issues errors about bad_p conversions at the beginning >>> of the function, but in the ck_ref_bind case, it only issues them >>> after we've called convert_like on the next conversion. >>> >>> This doesn't work as expected since r10-7096 because when we see >>> a conversion from/to class type in a template, we return early, thereby >>> missing the error, and a bad_p conversion goes by undetected. That >>> made the attached test to compile even though it should not. >> >> Hmm, why isn't there an error at instantiation time? > > We threw away the result: we're called from > > 12213 if (complain & tf_error) > 12214 { > 12215 if (conv) > 12216 convert_like (conv, expr, complain); > ... > 12228 return error_mark_node; > > and convert_like never saw converting this->f to B& again when instantiating. > >> Though giving an error at template parsing time is definitely preferable. > > Yup. > >>> I had thought that I could just move the ck_ref_bind/bad_p errors >>> above to the rest of them, but that regressed diagnostics because >>> expr then wasn't converted yet by the nested convert_like_real call. >> >> Yeah, the early section is really just for scalar conversions. >> >> It would probably be good to do normal processing for all other bad >> conversions and only afterward build the IMPLICIT_CONV_EXPR if we aren't >> returning error_mark_node. > > Ok, so that if we add more bad_p errors, we won't run into this again. > > Unfortunately it's a bit ugly. I could introduce a RETURN macro to > use RETURN (expr); instead of what I have now, but it wouldn't be simply > "conv_expr ? conv_expr : expr", because if we have error_mark_node, we > want to return that, not conv_expr. Does that seem worth it? > (I wish I could at least use the op0 ?: op1 GNU extension.) Or introduce a wrapper function that handles IMPLICIT_CONV_EXPR? > I've also added another test. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > convert_like issues errors about bad_p conversions at the beginning > of the function, but in the ck_ref_bind case, it only issues them > after we've called convert_like on the next conversion. > > This doesn't work as expected since r10-7096 because when we see > a conversion from/to class type in a template, we return early, thereby > missing the error, and a bad_p conversion goes by undetected. That > made the attached test to compile even though it should not. > > I had thought that I could just move the ck_ref_bind/bad_p errors > above to the rest of them, but that regressed diagnostics because > expr then wasn't converted yet by the nested convert_like_real call. > > So, for bad_p conversions, do the normal processing, but still return > the IMPLICIT_CONV_EXPR to avoid introducing trees that the template > processing can't handle well. > > gcc/cp/ChangeLog: > > PR c++/95789 > * call.c (convert_like_real): Do the normal processing for > conversion that are bad_p. Return the IMPLICIT_CONV_EXPR > instead of EXPR if we're processing a bad_p conversion in > a template. > > gcc/testsuite/ChangeLog: > > PR c++/95789 > * g++.dg/conversion/ref4.C: New test. > * g++.dg/conversion/ref5.C: New test. > --- > gcc/cp/call.c | 47 +++++++++++++++----------- > gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++ > gcc/testsuite/g++.dg/conversion/ref5.C | 14 ++++++++ > 3 files changed, 64 insertions(+), 19 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C > create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 5341a572980..65565fc90a8 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -7400,12 +7400,19 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > so use an IMPLICIT_CONV_EXPR for this conversion. We would have > created such codes e.g. when calling a user-defined conversion > function. */ > + tree conv_expr = NULL_TREE; > if (processing_template_decl > && convs->kind != ck_identity > && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))) > { > - expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); > - return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); > + conv_expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); > + if (convs->kind != ck_ref_bind) > + conv_expr = convert_from_reference (conv_expr); > + if (!convs->bad_p) > + return conv_expr; > + /* Do the normal processing to give the bad_p errors. But we still > + need to return the IMPLICIT_CONV_EXPR, unless we're returning > + error_mark_node. */ > } > > switch (convs->kind) > @@ -7465,7 +7472,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > TARGET_EXPR_LIST_INIT_P (expr) = true; > TARGET_EXPR_DIRECT_INIT_P (expr) = direct; > } > - return expr; > + return conv_expr ? conv_expr : expr; > } > > /* We don't know here whether EXPR is being used as an lvalue or > @@ -7488,7 +7495,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > TARGET_EXPR_LIST_INIT_P (expr) = true; > } > > - return expr; > + return conv_expr ? conv_expr : expr; > } > case ck_identity: > if (BRACE_ENCLOSED_INITIALIZER_P (expr)) > @@ -7513,7 +7520,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > continue to warn about uses of EXPR as an integer, rather than as a > pointer. */ > expr = build_int_cst (totype, 0); > - return expr; > + return conv_expr ? conv_expr : expr; > case ck_ambig: > /* We leave bad_p off ck_ambig because overload resolution considers > it valid, it just fails when we try to perform it. So we need to > @@ -7585,7 +7592,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > field = next_initializable_field (DECL_CHAIN (field)); > CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len)); > tree new_ctor = build_constructor (totype, vec); > - return get_target_expr_sfinae (new_ctor, complain); > + return (conv_expr ? conv_expr > + : get_target_expr_sfinae (new_ctor, complain)); > } > > case ck_aggr: > @@ -7598,14 +7606,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > imag = perform_implicit_conversion (TREE_TYPE (totype), > imag, complain); > expr = build2 (COMPLEX_EXPR, totype, real, imag); > - return expr; > + return conv_expr ? conv_expr : expr; > } > expr = reshape_init (totype, expr, complain); > expr = get_target_expr_sfinae (digest_init (totype, expr, complain), > complain); > if (expr != error_mark_node) > TARGET_EXPR_LIST_INIT_P (expr) = true; > - return expr; > + return conv_expr ? conv_expr : expr; > > default: > break; > @@ -7634,19 +7642,19 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > } > > if (! MAYBE_CLASS_TYPE_P (totype)) > - return expr; > + return conv_expr ? conv_expr : expr; > > /* Don't introduce copies when passing arguments along to the inherited > constructor. */ > if (current_function_decl > && flag_new_inheriting_ctors > && DECL_INHERITED_CTOR (current_function_decl)) > - return expr; > + return conv_expr ? conv_expr : expr; > > if (TREE_CODE (expr) == TARGET_EXPR > && TARGET_EXPR_LIST_INIT_P (expr)) > /* Copy-list-initialization doesn't actually involve a copy. */ > - return expr; > + return conv_expr ? conv_expr : expr; > > /* Fall through. */ > case ck_base: > @@ -7657,7 +7665,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > /* Build an expression for `*((base*) &expr)'. */ > expr = convert_to_base (expr, totype, > !c_cast_p, /*nonnull=*/true, complain); > - return expr; > + return conv_expr ? conv_expr : expr; > } > > /* Copy-initialization where the cv-unqualified version of the source > @@ -7686,7 +7694,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum); > } > > - return build_cplus_new (totype, expr, complain); > + return conv_expr ? conv_expr : build_cplus_new (totype, expr, complain); > > case ck_ref_bind: > { > @@ -7821,16 +7829,16 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > expr = cp_convert (build_pointer_type (TREE_TYPE (ref_type)), > expr, complain); > /* Convert the pointer to the desired reference type. */ > - return build_nop (ref_type, expr); > + return conv_expr ? conv_expr : build_nop (ref_type, expr); > } > > case ck_lvalue: > - return decay_conversion (expr, complain); > + return conv_expr ? conv_expr : decay_conversion (expr, complain); > > case ck_fnptr: > /* ??? Should the address of a transaction-safe pointer point to the TM > clone, and this conversion look up the primary function? */ > - return build_nop (totype, expr); > + return conv_expr ? conv_expr : build_nop (totype, expr); > > case ck_qual: > /* Warn about deprecated conversion if appropriate. */ > @@ -7845,11 +7853,12 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > if (convs->base_p) > expr = convert_to_base (expr, totype, !c_cast_p, > /*nonnull=*/false, complain); > - return build_nop (totype, expr); > + return conv_expr ? conv_expr : build_nop (totype, expr); > > case ck_pmem: > - return convert_ptrmem (totype, expr, /*allow_inverse_p=*/false, > + expr = convert_ptrmem (totype, expr, /*allow_inverse_p=*/false, > c_cast_p, complain); > + return conv_expr ? conv_expr : expr; > > default: > break; > @@ -7866,7 +7875,7 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > else > expr = cp_convert (totype, expr, complain); > > - return expr; > + return conv_expr ? conv_expr : expr; > } > > /* ARG is being passed to a varargs function. Perform any conversions > diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C > new file mode 100644 > index 00000000000..464a4cf6c0f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref4.C > @@ -0,0 +1,22 @@ > +// PR c++/95789 > +// { dg-do compile { target c++11 } } > + > +struct B { > + int n; > +}; > + > +template <typename T> > +struct A { > + B& get() const { return f; } // { dg-error "binding reference" } > + > + B f; > +}; > + > +int main() { > + A<int> a; > + a.f = {}; > + > + a.get().n = 10; > + if (a.f.n != 0) > + __builtin_abort(); > +} > diff --git a/gcc/testsuite/g++.dg/conversion/ref5.C b/gcc/testsuite/g++.dg/conversion/ref5.C > new file mode 100644 > index 00000000000..0042acd0670 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref5.C > @@ -0,0 +1,14 @@ > +// PR c++/96104 > + > +template <typename T> void fn(T &); > +class E {}; > +struct F { > + template <typename T> void mfn(T t) { t, fn(E()); } // { dg-error "cannot bind non-const lvalue reference" } > +}; > +int > +main() > +{ > + E e; > + F f; > + f.mfn(e); > +} > > base-commit: 760df6d296b8fc59796f42dca5eb14012fbfa28b > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-07-11 14:32 ` Jason Merrill @ 2020-07-13 20:48 ` Marek Polacek 2020-07-14 13:20 ` Jason Merrill 2020-07-14 19:37 ` Nathan Sidwell 0 siblings, 2 replies; 8+ messages in thread From: Marek Polacek @ 2020-07-13 20:48 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Sat, Jul 11, 2020 at 10:32:55AM -0400, Jason Merrill via Gcc-patches wrote: > On 7/8/20 4:35 PM, Marek Polacek wrote: > > On Fri, Jul 03, 2020 at 05:24:34PM -0400, Jason Merrill via Gcc-patches wrote: > > > On 6/22/20 10:09 PM, Marek Polacek wrote: > > > > convert_like issues errors about bad_p conversions at the beginning > > > > of the function, but in the ck_ref_bind case, it only issues them > > > > after we've called convert_like on the next conversion. > > > > > > > > This doesn't work as expected since r10-7096 because when we see > > > > a conversion from/to class type in a template, we return early, thereby > > > > missing the error, and a bad_p conversion goes by undetected. That > > > > made the attached test to compile even though it should not. > > > > > > Hmm, why isn't there an error at instantiation time? > > > > We threw away the result: we're called from > > > > 12213 if (complain & tf_error) > > 12214 { > > 12215 if (conv) > > 12216 convert_like (conv, expr, complain); > > ... > > 12228 return error_mark_node; > > > > and convert_like never saw converting this->f to B& again when instantiating. > > > > > Though giving an error at template parsing time is definitely preferable. > > > > Yup. > > > > > > I had thought that I could just move the ck_ref_bind/bad_p errors > > > > above to the rest of them, but that regressed diagnostics because > > > > expr then wasn't converted yet by the nested convert_like_real call. > > > > > > Yeah, the early section is really just for scalar conversions. > > > > > > It would probably be good to do normal processing for all other bad > > > conversions and only afterward build the IMPLICIT_CONV_EXPR if we aren't > > > returning error_mark_node. > > > > Ok, so that if we add more bad_p errors, we won't run into this again. > > > > Unfortunately it's a bit ugly. I could introduce a RETURN macro to > > use RETURN (expr); instead of what I have now, but it wouldn't be simply > > "conv_expr ? conv_expr : expr", because if we have error_mark_node, we > > want to return that, not conv_expr. Does that seem worth it? > > (I wish I could at least use the op0 ?: op1 GNU extension.) > > Or introduce a wrapper function that handles IMPLICIT_CONV_EXPR? Perhaps like this? convert_like_real_1 is unsightly and I have plans to fix that (and get rid of the convert_like macro), but that's a clean-up, not a bug fix so it's not included in this patch. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? -- >8 -- convert_like issues errors about bad_p conversions at the beginning of the function, but in the ck_ref_bind case, it only issues them after we've called convert_like on the next conversion. This doesn't work as expected since r10-7096 because when we see a conversion from/to class type in a template, we return early, thereby missing the error, and a bad_p conversion goes by undetected. That made the attached test to compile even though it should not. I had thought that I could just move the ck_ref_bind/bad_p errors above to the rest of them, but that regressed diagnostics because expr then wasn't converted yet by the nested convert_like_real call. So, for bad_p conversions, do the normal processing, but still return the IMPLICIT_CONV_EXPR to avoid introducing trees that the template processing can't handle well. This I achieved by adding a wrapper function. gcc/cp/ChangeLog: PR c++/95789 PR c++/96104 PR c++/96179 * call.c (convert_like_real_1): Renamed from convert_like_real. (convert_like_real): New wrapper for convert_like_real_1. gcc/testsuite/ChangeLog: PR c++/95789 PR c++/96104 PR c++/96179 * g++.dg/conversion/ref4.C: New test. * g++.dg/conversion/ref5.C: New test. * g++.dg/conversion/ref6.C: New test. --- gcc/cp/call.c | 54 ++++++++++++++++++-------- gcc/testsuite/g++.dg/conversion/ref4.C | 22 +++++++++++ gcc/testsuite/g++.dg/conversion/ref5.C | 14 +++++++ gcc/testsuite/g++.dg/conversion/ref6.C | 24 ++++++++++++ 4 files changed, 98 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C create mode 100644 gcc/testsuite/g++.dg/conversion/ref6.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 5341a572980..6d5d5e801a5 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -171,6 +171,8 @@ static tree build_over_call (struct z_candidate *, int, tsubst_flags_t); /*c_cast_p=*/false, (COMPLAIN)) static tree convert_like_real (conversion *, tree, tree, int, bool, bool, tsubst_flags_t); +static tree convert_like_real_1 (conversion *, tree, tree, int, bool, + bool, tsubst_flags_t); static void op_error (const op_location_t &, enum tree_code, enum tree_code, tree, tree, tree, bool); static struct z_candidate *build_user_type_conversion_1 (tree, tree, int, @@ -7281,6 +7283,39 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr) "are only available with %<-std=c++20%> or %<-std=gnu++20%>"); } +/* Wrapper for convert_like_real_1 that handles creating IMPLICIT_CONV_EXPR. */ + +static tree +convert_like_real (conversion *convs, tree expr, tree fn, int argnum, + bool issue_conversion_warnings, + bool c_cast_p, tsubst_flags_t complain) +{ + /* 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. */ + tree conv_expr = NULL_TREE; + if (processing_template_decl + && convs->kind != ck_identity + && (CLASS_TYPE_P (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr)))) + { + conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, expr); + if (convs->kind != ck_ref_bind) + conv_expr = convert_from_reference (conv_expr); + if (!convs->bad_p) + return conv_expr; + /* Do the normal processing to give the bad_p errors. But we still + need to return the IMPLICIT_CONV_EXPR, unless we're returning + error_mark_node. */ + } + expr = convert_like_real_1 (convs, expr, fn, argnum, + issue_conversion_warnings, c_cast_p, complain); + if (expr == error_mark_node) + return error_mark_node; + return conv_expr ? conv_expr : expr; +} + /* Perform the conversions in CONVS on the expression EXPR. FN and ARGNUM are used for diagnostics. ARGNUM is zero based, -1 indicates the `this' argument of a method. INNER is nonzero when @@ -7292,9 +7327,9 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr) conversions to inaccessible bases are permitted. */ static tree -convert_like_real (conversion *convs, tree expr, tree fn, int argnum, - bool issue_conversion_warnings, - bool c_cast_p, tsubst_flags_t complain) +convert_like_real_1 (conversion *convs, tree expr, tree fn, int argnum, + bool issue_conversion_warnings, + bool c_cast_p, tsubst_flags_t complain) { tree totype = convs->type; diagnostic_t diag_kind; @@ -7395,19 +7430,6 @@ 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)))) - { - expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); - return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); - } - switch (convs->kind) { case ck_user: diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C new file mode 100644 index 00000000000..464a4cf6c0f --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/ref4.C @@ -0,0 +1,22 @@ +// PR c++/95789 +// { dg-do compile { target c++11 } } + +struct B { + int n; +}; + +template <typename T> +struct A { + B& get() const { return f; } // { dg-error "binding reference" } + + B f; +}; + +int main() { + A<int> a; + a.f = {}; + + a.get().n = 10; + if (a.f.n != 0) + __builtin_abort(); +} diff --git a/gcc/testsuite/g++.dg/conversion/ref5.C b/gcc/testsuite/g++.dg/conversion/ref5.C new file mode 100644 index 00000000000..0042acd0670 --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/ref5.C @@ -0,0 +1,14 @@ +// PR c++/96104 + +template <typename T> void fn(T &); +class E {}; +struct F { + template <typename T> void mfn(T t) { t, fn(E()); } // { dg-error "cannot bind non-const lvalue reference" } +}; +int +main() +{ + E e; + F f; + f.mfn(e); +} diff --git a/gcc/testsuite/g++.dg/conversion/ref6.C b/gcc/testsuite/g++.dg/conversion/ref6.C new file mode 100644 index 00000000000..fc87199053c --- /dev/null +++ b/gcc/testsuite/g++.dg/conversion/ref6.C @@ -0,0 +1,24 @@ +// PR c++/96179 +// { dg-do compile { target c++11 } } + +template<typename T> struct vector +{ + void push_back(T) { } +}; + +struct dummy{ + int a; +}; + +void Modify_Dummy(dummy &d){ + d.a=1; +} + +template <bool bla=true> void Templated_Function(){ + vector<dummy> A; + A.push_back(Modify_Dummy(dummy{0})); // { dg-error "cannot bind non-const lvalue reference" } +} + +int main(){ + Templated_Function(); +} base-commit: 9cba898481368ce16c6a2d30ef781a82dce27c55 -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-07-13 20:48 ` [PATCH v3] " Marek Polacek @ 2020-07-14 13:20 ` Jason Merrill 2020-07-14 19:37 ` Nathan Sidwell 1 sibling, 0 replies; 8+ messages in thread From: Jason Merrill @ 2020-07-14 13:20 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 7/13/20 4:48 PM, Marek Polacek wrote: > On Sat, Jul 11, 2020 at 10:32:55AM -0400, Jason Merrill via Gcc-patches wrote: >> On 7/8/20 4:35 PM, Marek Polacek wrote: >>> On Fri, Jul 03, 2020 at 05:24:34PM -0400, Jason Merrill via Gcc-patches wrote: >>>> On 6/22/20 10:09 PM, Marek Polacek wrote: >>>>> convert_like issues errors about bad_p conversions at the beginning >>>>> of the function, but in the ck_ref_bind case, it only issues them >>>>> after we've called convert_like on the next conversion. >>>>> >>>>> This doesn't work as expected since r10-7096 because when we see >>>>> a conversion from/to class type in a template, we return early, thereby >>>>> missing the error, and a bad_p conversion goes by undetected. That >>>>> made the attached test to compile even though it should not. >>>> >>>> Hmm, why isn't there an error at instantiation time? >>> >>> We threw away the result: we're called from >>> >>> 12213 if (complain & tf_error) >>> 12214 { >>> 12215 if (conv) >>> 12216 convert_like (conv, expr, complain); >>> ... >>> 12228 return error_mark_node; >>> >>> and convert_like never saw converting this->f to B& again when instantiating. >>> >>>> Though giving an error at template parsing time is definitely preferable. >>> >>> Yup. >>> >>>>> I had thought that I could just move the ck_ref_bind/bad_p errors >>>>> above to the rest of them, but that regressed diagnostics because >>>>> expr then wasn't converted yet by the nested convert_like_real call. >>>> >>>> Yeah, the early section is really just for scalar conversions. >>>> >>>> It would probably be good to do normal processing for all other bad >>>> conversions and only afterward build the IMPLICIT_CONV_EXPR if we aren't >>>> returning error_mark_node. >>> >>> Ok, so that if we add more bad_p errors, we won't run into this again. >>> >>> Unfortunately it's a bit ugly. I could introduce a RETURN macro to >>> use RETURN (expr); instead of what I have now, but it wouldn't be simply >>> "conv_expr ? conv_expr : expr", because if we have error_mark_node, we >>> want to return that, not conv_expr. Does that seem worth it? >>> (I wish I could at least use the op0 ?: op1 GNU extension.) >> >> Or introduce a wrapper function that handles IMPLICIT_CONV_EXPR? > > Perhaps like this? convert_like_real_1 is unsightly and I have > plans to fix that (and get rid of the convert_like macro), but > that's a clean-up, not a bug fix so it's not included in this patch. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? OK. > -- >8 -- > convert_like issues errors about bad_p conversions at the beginning > of the function, but in the ck_ref_bind case, it only issues them > after we've called convert_like on the next conversion. > > This doesn't work as expected since r10-7096 because when we see > a conversion from/to class type in a template, we return early, thereby > missing the error, and a bad_p conversion goes by undetected. That > made the attached test to compile even though it should not. > > I had thought that I could just move the ck_ref_bind/bad_p errors > above to the rest of them, but that regressed diagnostics because > expr then wasn't converted yet by the nested convert_like_real call. > > So, for bad_p conversions, do the normal processing, but still return > the IMPLICIT_CONV_EXPR to avoid introducing trees that the template > processing can't handle well. This I achieved by adding a wrapper > function. > > gcc/cp/ChangeLog: > > PR c++/95789 > PR c++/96104 > PR c++/96179 > * call.c (convert_like_real_1): Renamed from convert_like_real. > (convert_like_real): New wrapper for convert_like_real_1. > > gcc/testsuite/ChangeLog: > > PR c++/95789 > PR c++/96104 > PR c++/96179 > * g++.dg/conversion/ref4.C: New test. > * g++.dg/conversion/ref5.C: New test. > * g++.dg/conversion/ref6.C: New test. > --- > gcc/cp/call.c | 54 ++++++++++++++++++-------- > gcc/testsuite/g++.dg/conversion/ref4.C | 22 +++++++++++ > gcc/testsuite/g++.dg/conversion/ref5.C | 14 +++++++ > gcc/testsuite/g++.dg/conversion/ref6.C | 24 ++++++++++++ > 4 files changed, 98 insertions(+), 16 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C > create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C > create mode 100644 gcc/testsuite/g++.dg/conversion/ref6.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 5341a572980..6d5d5e801a5 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -171,6 +171,8 @@ static tree build_over_call (struct z_candidate *, int, tsubst_flags_t); > /*c_cast_p=*/false, (COMPLAIN)) > static tree convert_like_real (conversion *, tree, tree, int, bool, > bool, tsubst_flags_t); > +static tree convert_like_real_1 (conversion *, tree, tree, int, bool, > + bool, tsubst_flags_t); > static void op_error (const op_location_t &, enum tree_code, enum tree_code, > tree, tree, tree, bool); > static struct z_candidate *build_user_type_conversion_1 (tree, tree, int, > @@ -7281,6 +7283,39 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr) > "are only available with %<-std=c++20%> or %<-std=gnu++20%>"); > } > > +/* Wrapper for convert_like_real_1 that handles creating IMPLICIT_CONV_EXPR. */ > + > +static tree > +convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > + bool issue_conversion_warnings, > + bool c_cast_p, tsubst_flags_t complain) > +{ > + /* 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. */ > + tree conv_expr = NULL_TREE; > + if (processing_template_decl > + && convs->kind != ck_identity > + && (CLASS_TYPE_P (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr)))) > + { > + conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, expr); > + if (convs->kind != ck_ref_bind) > + conv_expr = convert_from_reference (conv_expr); > + if (!convs->bad_p) > + return conv_expr; > + /* Do the normal processing to give the bad_p errors. But we still > + need to return the IMPLICIT_CONV_EXPR, unless we're returning > + error_mark_node. */ > + } > + expr = convert_like_real_1 (convs, expr, fn, argnum, > + issue_conversion_warnings, c_cast_p, complain); > + if (expr == error_mark_node) > + return error_mark_node; > + return conv_expr ? conv_expr : expr; > +} > + > /* Perform the conversions in CONVS on the expression EXPR. FN and > ARGNUM are used for diagnostics. ARGNUM is zero based, -1 > indicates the `this' argument of a method. INNER is nonzero when > @@ -7292,9 +7327,9 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr) > conversions to inaccessible bases are permitted. */ > > static tree > -convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > - bool issue_conversion_warnings, > - bool c_cast_p, tsubst_flags_t complain) > +convert_like_real_1 (conversion *convs, tree expr, tree fn, int argnum, > + bool issue_conversion_warnings, > + bool c_cast_p, tsubst_flags_t complain) > { > tree totype = convs->type; > diagnostic_t diag_kind; > @@ -7395,19 +7430,6 @@ 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)))) > - { > - expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); > - return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); > - } > - > switch (convs->kind) > { > case ck_user: > diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C > new file mode 100644 > index 00000000000..464a4cf6c0f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref4.C > @@ -0,0 +1,22 @@ > +// PR c++/95789 > +// { dg-do compile { target c++11 } } > + > +struct B { > + int n; > +}; > + > +template <typename T> > +struct A { > + B& get() const { return f; } // { dg-error "binding reference" } > + > + B f; > +}; > + > +int main() { > + A<int> a; > + a.f = {}; > + > + a.get().n = 10; > + if (a.f.n != 0) > + __builtin_abort(); > +} > diff --git a/gcc/testsuite/g++.dg/conversion/ref5.C b/gcc/testsuite/g++.dg/conversion/ref5.C > new file mode 100644 > index 00000000000..0042acd0670 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref5.C > @@ -0,0 +1,14 @@ > +// PR c++/96104 > + > +template <typename T> void fn(T &); > +class E {}; > +struct F { > + template <typename T> void mfn(T t) { t, fn(E()); } // { dg-error "cannot bind non-const lvalue reference" } > +}; > +int > +main() > +{ > + E e; > + F f; > + f.mfn(e); > +} > diff --git a/gcc/testsuite/g++.dg/conversion/ref6.C b/gcc/testsuite/g++.dg/conversion/ref6.C > new file mode 100644 > index 00000000000..fc87199053c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref6.C > @@ -0,0 +1,24 @@ > +// PR c++/96179 > +// { dg-do compile { target c++11 } } > + > +template<typename T> struct vector > +{ > + void push_back(T) { } > +}; > + > +struct dummy{ > + int a; > +}; > + > +void Modify_Dummy(dummy &d){ > + d.a=1; > +} > + > +template <bool bla=true> void Templated_Function(){ > + vector<dummy> A; > + A.push_back(Modify_Dummy(dummy{0})); // { dg-error "cannot bind non-const lvalue reference" } > +} > + > +int main(){ > + Templated_Function(); > +} > > base-commit: 9cba898481368ce16c6a2d30ef781a82dce27c55 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] 2020-07-13 20:48 ` [PATCH v3] " Marek Polacek 2020-07-14 13:20 ` Jason Merrill @ 2020-07-14 19:37 ` Nathan Sidwell 1 sibling, 0 replies; 8+ messages in thread From: Nathan Sidwell @ 2020-07-14 19:37 UTC (permalink / raw) To: Marek Polacek, Jason Merrill; +Cc: GCC Patches On 7/13/20 4:48 PM, Marek Polacek via Gcc-patches wrote: > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > -- >8 -- > convert_like issues errors about bad_p conversions at the beginning > of the function, but in the ck_ref_bind case, it only issues them > after we've called convert_like on the next conversion. > > This doesn't work as expected since r10-7096 because when we see > a conversion from/to class type in a template, we return early, thereby > missing the error, and a bad_p conversion goes by undetected. That > made the attached test to compile even though it should not. > > I had thought that I could just move the ck_ref_bind/bad_p errors > above to the rest of them, but that regressed diagnostics because > expr then wasn't converted yet by the nested convert_like_real call. > > So, for bad_p conversions, do the normal processing, but still return > the IMPLICIT_CONV_EXPR to avoid introducing trees that the template > processing can't handle well. This I achieved by adding a wrapper > function. LGTM, thanks! > > gcc/cp/ChangeLog: > > PR c++/95789 > PR c++/96104 > PR c++/96179 > * call.c (convert_like_real_1): Renamed from convert_like_real. > (convert_like_real): New wrapper for convert_like_real_1. > > gcc/testsuite/ChangeLog: > > PR c++/95789 > PR c++/96104 > PR c++/96179 > * g++.dg/conversion/ref4.C: New test. > * g++.dg/conversion/ref5.C: New test. > * g++.dg/conversion/ref6.C: New test. > --- > gcc/cp/call.c | 54 ++++++++++++++++++-------- > gcc/testsuite/g++.dg/conversion/ref4.C | 22 +++++++++++ > gcc/testsuite/g++.dg/conversion/ref5.C | 14 +++++++ > gcc/testsuite/g++.dg/conversion/ref6.C | 24 ++++++++++++ > 4 files changed, 98 insertions(+), 16 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C > create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C > create mode 100644 gcc/testsuite/g++.dg/conversion/ref6.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 5341a572980..6d5d5e801a5 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -171,6 +171,8 @@ static tree build_over_call (struct z_candidate *, int, tsubst_flags_t); > /*c_cast_p=*/false, (COMPLAIN)) > static tree convert_like_real (conversion *, tree, tree, int, bool, > bool, tsubst_flags_t); > +static tree convert_like_real_1 (conversion *, tree, tree, int, bool, > + bool, tsubst_flags_t); > static void op_error (const op_location_t &, enum tree_code, enum tree_code, > tree, tree, tree, bool); > static struct z_candidate *build_user_type_conversion_1 (tree, tree, int, > @@ -7281,6 +7283,39 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr) > "are only available with %<-std=c++20%> or %<-std=gnu++20%>"); > } > > +/* Wrapper for convert_like_real_1 that handles creating IMPLICIT_CONV_EXPR. */ > + > +static tree > +convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > + bool issue_conversion_warnings, > + bool c_cast_p, tsubst_flags_t complain) > +{ > + /* 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. */ > + tree conv_expr = NULL_TREE; > + if (processing_template_decl > + && convs->kind != ck_identity > + && (CLASS_TYPE_P (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr)))) > + { > + conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, expr); > + if (convs->kind != ck_ref_bind) > + conv_expr = convert_from_reference (conv_expr); > + if (!convs->bad_p) > + return conv_expr; > + /* Do the normal processing to give the bad_p errors. But we still > + need to return the IMPLICIT_CONV_EXPR, unless we're returning > + error_mark_node. */ > + } > + expr = convert_like_real_1 (convs, expr, fn, argnum, > + issue_conversion_warnings, c_cast_p, complain); > + if (expr == error_mark_node) > + return error_mark_node; > + return conv_expr ? conv_expr : expr; > +} > + > /* Perform the conversions in CONVS on the expression EXPR. FN and > ARGNUM are used for diagnostics. ARGNUM is zero based, -1 > indicates the `this' argument of a method. INNER is nonzero when > @@ -7292,9 +7327,9 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr) > conversions to inaccessible bases are permitted. */ > > static tree > -convert_like_real (conversion *convs, tree expr, tree fn, int argnum, > - bool issue_conversion_warnings, > - bool c_cast_p, tsubst_flags_t complain) > +convert_like_real_1 (conversion *convs, tree expr, tree fn, int argnum, > + bool issue_conversion_warnings, > + bool c_cast_p, tsubst_flags_t complain) > { > tree totype = convs->type; > diagnostic_t diag_kind; > @@ -7395,19 +7430,6 @@ 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)))) > - { > - expr = build1 (IMPLICIT_CONV_EXPR, totype, expr); > - return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr); > - } > - > switch (convs->kind) > { > case ck_user: > diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C b/gcc/testsuite/g++.dg/conversion/ref4.C > new file mode 100644 > index 00000000000..464a4cf6c0f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref4.C > @@ -0,0 +1,22 @@ > +// PR c++/95789 > +// { dg-do compile { target c++11 } } > + > +struct B { > + int n; > +}; > + > +template <typename T> > +struct A { > + B& get() const { return f; } // { dg-error "binding reference" } > + > + B f; > +}; > + > +int main() { > + A<int> a; > + a.f = {}; > + > + a.get().n = 10; > + if (a.f.n != 0) > + __builtin_abort(); > +} > diff --git a/gcc/testsuite/g++.dg/conversion/ref5.C b/gcc/testsuite/g++.dg/conversion/ref5.C > new file mode 100644 > index 00000000000..0042acd0670 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref5.C > @@ -0,0 +1,14 @@ > +// PR c++/96104 > + > +template <typename T> void fn(T &); > +class E {}; > +struct F { > + template <typename T> void mfn(T t) { t, fn(E()); } // { dg-error "cannot bind non-const lvalue reference" } > +}; > +int > +main() > +{ > + E e; > + F f; > + f.mfn(e); > +} > diff --git a/gcc/testsuite/g++.dg/conversion/ref6.C b/gcc/testsuite/g++.dg/conversion/ref6.C > new file mode 100644 > index 00000000000..fc87199053c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref6.C > @@ -0,0 +1,24 @@ > +// PR c++/96179 > +// { dg-do compile { target c++11 } } > + > +template<typename T> struct vector > +{ > + void push_back(T) { } > +}; > + > +struct dummy{ > + int a; > +}; > + > +void Modify_Dummy(dummy &d){ > + d.a=1; > +} > + > +template <bool bla=true> void Templated_Function(){ > + vector<dummy> A; > + A.push_back(Modify_Dummy(dummy{0})); // { dg-error "cannot bind non-const lvalue reference" } > +} > + > +int main(){ > + Templated_Function(); > +} > > base-commit: 9cba898481368ce16c6a2d30ef781a82dce27c55 > -- Nathan Sidwell ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-17 15:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-23 2:09 [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789] Marek Polacek 2020-06-29 15:01 ` Marek Polacek 2020-07-03 21:24 ` Jason Merrill 2020-07-08 20:35 ` [PATCH v2] " Marek Polacek 2020-07-11 14:32 ` Jason Merrill 2020-07-13 20:48 ` [PATCH v3] " Marek Polacek 2020-07-14 13:20 ` Jason Merrill 2020-07-14 19:37 ` Nathan Sidwell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).