Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Nathan Sidwell <nathan@acm.org>]

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