[C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

Hi,

in this C++17 ICE on invalid we crash when, in do_class_deduction, 
build_new_function_call is called with a null first argument. What is 
happening is pretty simple to analyze: for the broken snippet, there are 
no cands and elided is false, because the issue isn't that candidates 
are elided because explicit, instead that there are no viable candidates 
whatsoever, because args->length () == 3 and we don't even have the 
implicit deduction guides. Alternately to the straightforward fix I'm 
proposing below, a literal reading of the standard [16.3.1.8] suggests 
that we could also consider generating the implicit deduction guides 
even when args->length () >= 2 and the copy deduction guide even when 
args->length () != 1, that is unconditionally.

While working on the bug I also noticed that we can simplify a bit the 
code generating the implicit deduction guides: if I'm not mistaken, when 
we pass types as first argument of build_deduction_guide - for implicit 
guides, that is - the deduction guide is never explicit. thus 
DECL_NONCONVERTING_P is never true. It's an unrelated tweak, anyway, 
which we can consider applying by itself if we don't change the code 
generating the implicit deduction guides.

Thanks! Paolo.

///////////////////////


[-- Attachment #2: CL_79790 --]
[-- Type: text/plain, Size: 340 bytes --]

/cp
2017-07-14  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/79790
	* pt.c (do_class_deduction): Handle the case of no viable implicit
	deduction guides; simplify the code generating implicit deduction
	guides.

/testsuite
2017-07-14  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/79790
	* g++.dg/cpp1z/class-deduction42.C: New.

[-- Attachment #3: patch_79790 --]
[-- Type: text/plain, Size: 1383 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 250186)
+++ cp/pt.c	(working copy)
@@ -25439,11 +25439,7 @@ do_class_deduction (tree ptype, tree tmpl, tree in
       if (gtype)
 	{
 	  tree guide = build_deduction_guide (gtype, outer_args, complain);
-	  if ((flags & LOOKUP_ONLYCONVERTING)
-	      && DECL_NONCONVERTING_P (STRIP_TEMPLATE (guide)))
-	    elided = true;
-	  else
-	    cands = lookup_add (guide, cands);
+	  cands = lookup_add (guide, cands);
 	}
     }
 
@@ -25454,6 +25450,12 @@ do_class_deduction (tree ptype, tree tmpl, tree in
 	     "user-declared constructors", type);
       return error_mark_node;
     }
+  else if (!cands && call == error_mark_node)
+    {
+      error ("cannot deduce template arguments for copy-initialization"
+	     " of %qT, as it has no viable implicit deduction guides", type);
+      return error_mark_node;
+    }
 
   if (call == error_mark_node)
     {
Index: testsuite/g++.dg/cpp1z/class-deduction42.C
===================================================================
--- testsuite/g++.dg/cpp1z/class-deduction42.C	(revision 0)
+++ testsuite/g++.dg/cpp1z/class-deduction42.C	(working copy)
@@ -0,0 +1,10 @@
+// PR c++/79790
+// { dg-options -std=c++1z }
+
+template <int N>
+struct array
+{
+  int a [N];
+};
+
+array a = { 1, 2, 3 };  // { dg-error "cannot deduce" }

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Nathan Sidwell]

On 07/14/2017 01:32 PM, Paolo Carlini wrote:

> While working on the bug I also noticed that we can simplify a bit the code
> generating the implicit deduction guides: if I'm not mistaken, when we pass
> types as first argument of build_deduction_guide - for implicit guides, that is
> - the deduction guide is never explicit. thus DECL_NONCONVERTING_P is never
> true. It's an unrelated tweak, anyway, which we can consider applying by itself
> if we don't change the code generating the implicit deduction guides.

I recall wondering the same thing when adding the 'elided = true' pieces, but 
didn't investigate enough to confirm/deny.  Thanks for getting this.

nathan

-- 
Nathan Sidwell

[C++ Patch Ping] Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

Hi,

On 14/07/2017 19:51, Nathan Sidwell wrote:
> On 07/14/2017 01:32 PM, Paolo Carlini wrote:
>
>> While working on the bug I also noticed that we can simplify a bit 
>> the code
>> generating the implicit deduction guides: if I'm not mistaken, when 
>> we pass
>> types as first argument of build_deduction_guide - for implicit 
>> guides, that is
>> - the deduction guide is never explicit. thus DECL_NONCONVERTING_P is 
>> never
>> true. It's an unrelated tweak, anyway, which we can consider applying 
>> by itself
>> if we don't change the code generating the implicit deduction guides.
>
> I recall wondering the same thing when adding the 'elided = true' 
> pieces, but didn't investigate enough to confirm/deny.  Thanks for 
> getting this.
You are welcome!

I think the rest of the patch - the bug fix proper - still awaits a 
review...

Thanks!
Paolo.

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Jason Merrill]

On 07/14/2017 01:32 PM, Paolo Carlini wrote:
> +      error ("cannot deduce template arguments for copy-initialization"
> +	     " of %qT, as it has no viable implicit deduction guides", type);

Why "copy-initialization"?  We do deduction for direct-initialization, 
as well.

I would also drop the "implicit".

OK with those tweaks.

Jason

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

Hi,

On 04/08/2017 19:00, Jason Merrill wrote:
> On 07/14/2017 01:32 PM, Paolo Carlini wrote:
>> +      error ("cannot deduce template arguments for copy-initialization"
>> +         " of %qT, as it has no viable implicit deduction guides", 
>> type);
>
> Why "copy-initialization"?  We do deduction for direct-initialization, 
> as well.
>
> I would also drop the "implicit".
>
> OK with those tweaks.
Thanks Jason for your review and, essentially, approval of the work.

About the exact wording, I'm a little puzzled, because, besides the 
"implicit" nit, the "copy-inizialization" already occurs in two other 
places in that function, in the preceding:

   if (elided && !cands)
     {
       error ("cannot deduce template arguments for copy-initialization"
          " of %qT, as it has no non-explicit deduction guides or "
          "user-declared constructors", type);
       return error_mark_node;
     }

and in the following:

       if (elided)
     inform (input_location, "explicit deduction guides not considered "
         "for copy-initialization");

and now I'm wondering which, if any, should be also removed?!? Both? Can 
you help me about that?

Thanks,
Paolo.

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Tim Song]

On Fri, Aug 4, 2017 at 1:32 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:

> About the exact wording, I'm a little puzzled, because, besides the
> "implicit" nit, the "copy-inizialization" already occurs in two other places
> in that function, in the preceding:
>
>   if (elided && !cands)
>     {
>       error ("cannot deduce template arguments for copy-initialization"
>          " of %qT, as it has no non-explicit deduction guides or "
>          "user-declared constructors", type);
>       return error_mark_node;
>     }
>
> and in the following:
>
>       if (elided)
>     inform (input_location, "explicit deduction guides not considered "
>         "for copy-initialization");
>
> and now I'm wondering which, if any, should be also removed?!? Both? Can you
> help me about that?
>
> Thanks,
> Paolo.

These messages are only emitted when elided == true, which, if I'm
reading the code correctly, is only the case when you are in
copy-initialization context (and have skipped at least one explicit
guide).

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

Hi,

On 04/08/2017 20:05, Tim Song wrote:
> These messages are only emitted when elided == true, which, if I'm
> reading the code correctly, is only the case when you are in
> copy-initialization context (and have skipped at least one explicit
> guide).
Ah! Thanks!

Paolo.