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


Reply via email to