[PR,c++/84979] improve auto handling in explicit tmpl args for concepts

Message ID orh8p6pop0.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [PR,c++/84979] improve auto handling in explicit tmpl args for concepts
Related show

Commit Message

Alexandre Oliva March 23, 2018, 7:38 p.m.
We fail to detect unresolved explicitly-passed auto template args.

The first hunk in pt.c, that changes fn_type_unification, arranges for
us to regard the template arg list as incomplete, although that's not
necessary for any of the testcases I tried.  I thought it might be
relevant in case the args become part of the type of the function, and
for us to continue overload resolution with processing_template_decl
nonzero.

The second hunk, that changes type_unification_real, is the actual fix
for the testcase in the PR.

The other hunks, that change unify, arrange for us to accept and
properly update the template arglist, at least when auto stands by
itself as a template argument.  Without this hunk, the slightly
modified testcase pr84979-2.C would fail, although there's no good
reason for the deduction to be regarded as a mismatch.

We still need some work, however: pr84979-3.C and pr84979-4.C should
be accepted, if we are to support non-toplevel auto in explicit
parameters (-3), or auto in parms whose index doesn't match the auto's
own index.


This is probably not good enough for inclusion, although I'm pretty sure
it passes regstrap.  The problem is the condition in unify() that
intends to allow proper uses of auto by do_auto_deduction, and the
AFAICT accidentally-working cases of auto deduction in explicit template
args: the condition that covers those cases is growing incredibly
hairier and fragile, and I'm growing convinced I took a wrong turn
somewhere that led me there.  I'm almost ready to conclude that I'm
either missing something, or that we shouldn't be supporting auto in
explicit template args at all, because I can't see that the
implementation was ever meant to support that use.  Is that so?  I could
use some advice there.  Thanks in advance,


for  gcc/cp/ChangeLog

	PR c++/84979
	* pt.c (fn_type_unification): Set incomplete if explicit args
	use auto.
	(type_unification_real): Do not skip deduction if explicit
	arg uses auto.
	(unify): Reject unsupported cases of auto in args.  Regard
	auto as a match, but proceed to update targ.

for  gcc/testsuite/ChangeLog

	PR c++/84979
	* g++.dg/concepts/pr84979.C: New.
	* g++.dg/concepts/pr84979-2.C: New.
	* g++.dg/concepts/pr84979-3.C: New.
	* g++.dg/concepts/pr84979-4.C: New.
---
 gcc/cp/pt.c                               |   56 +++++++++++++++++++++++++++--
 gcc/testsuite/g++.dg/concepts/pr84979-2.C |    9 +++++
 gcc/testsuite/g++.dg/concepts/pr84979-3.C |    9 +++++
 gcc/testsuite/g++.dg/concepts/pr84979-4.C |    9 +++++
 gcc/testsuite/g++.dg/concepts/pr84979.C   |    9 +++++
 5 files changed, 88 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-2.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-3.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-4.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Comments

Jason Merrill March 23, 2018, 7:48 p.m. | #1
On Fri, Mar 23, 2018 at 3:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> +  /* Concepts allows 'auto' in template arguments, even multiple

> +     'auto's in a single argument.


I think that's only intended for class templates; we should reject
'auto' as a template argument for function or variable templates.

Jason
Alexandre Oliva March 28, 2018, 9:06 a.m. | #2
On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Fri, Mar 23, 2018 at 3:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote:

>> +  /* Concepts allows 'auto' in template arguments, even multiple

>> +     'auto's in a single argument.


> I think that's only intended for class templates;


Aah, that explains a lot!  Dang, it was so close!

It actually makes sense; was there any discussion about standardizing
this, with reasons to reject this construct, or is it just that we
aren't there yet?  I wouldn't be surprised if it were to appear in some
future version of C++.

> we should reject 'auto' as a template argument for function or

> variable templates.


Is this (in the patch below) the best spot to test for it?

[PR c++/84979] reject auto in explicit tmpl args for tmpl-fn

With concepts, we accept auto in explicit template arguments, but we
should only accept them for template classes.  Passing them to
template functions is not allowed.  So, reject it.

Regstrapping on i686- and x86-64-linux-gnu, after a successful make
check-c++-all.  Ok to install if it passes?


for  gcc/cp/ChangeLog

	PR c++/84979
	* semantics.c (finish_id_expression): Reject template args
	referencing auto for template fns.

for  gcc/testsuite/ChangeLog

	PR c++/84979
	* g++.dg/concepts/pr84979.C: New.
---
 gcc/cp/semantics.c                      |   11 +++++++++--
 gcc/testsuite/g++.dg/concepts/pr84979.C |    9 +++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 035e39515748..9b33234c8b73 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3528,8 +3528,15 @@ finish_id_expression (tree id_expression,
   /* If we have a template-id, then no further lookup is
      required.  If the template-id was for a template-class, we
      will sometimes have a TYPE_DECL at this point.  */
-  else if (TREE_CODE (decl) == TEMPLATE_ID_EXPR
-	   || TREE_CODE (decl) == TYPE_DECL)
+  else if (TREE_CODE (decl) == TEMPLATE_ID_EXPR)
+    {
+      if (flag_concepts && type_uses_auto (decl))
+	{
+	  *error_msg = "invalid use of %<auto%> in template argument for template function";
+	  return error_mark_node;
+	}
+    }
+  else if (TREE_CODE (decl) == TYPE_DECL)
     ;
   /* Look up the name.  */
   else
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979.C b/gcc/testsuite/g++.dg/concepts/pr84979.C
new file mode 100644
index 000000000000..c70b3756f2b8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename> void foo() {}
+
+void bar()
+{
+  foo<auto>(); // { dg-error "invalid|no match" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill March 28, 2018, 6:35 p.m. | #3
On Wed, Mar 28, 2018 at 5:06 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>> On Fri, Mar 23, 2018 at 3:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>> +  /* Concepts allows 'auto' in template arguments, even multiple

>>> +     'auto's in a single argument.

>

>> I think that's only intended for class templates;

>

> Aah, that explains a lot!  Dang, it was so close!

>

> It actually makes sense; was there any discussion about standardizing

> this, with reasons to reject this construct, or is it just that we

> aren't there yet?  I wouldn't be surprised if it were to appear in some

> future version of C++.


If what were to appear?  I'm not sure what you mean.  There is still
effort to get more of the Concepts TS into the standard.

We only want 'auto' in places where it could be deduced, and there's
no way in [temp.deduct.type] to deduce from explicit template
arguments to a non-type template.

> Is this (in the patch below) the best spot to test for it?


This seems like a plausible spot, but is there a reason not to check
in cp_parser_template_id?

Jason
Alexandre Oliva March 29, 2018, 10:24 p.m. | #4
On Mar 28, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Wed, Mar 28, 2018 at 5:06 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>> On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote:

>> 

>>> On Fri, Mar 23, 2018 at 3:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>>> +  /* Concepts allows 'auto' in template arguments, even multiple

>>>> +     'auto's in a single argument.

>> 

>>> I think that's only intended for class templates;

>> 

>> Aah, that explains a lot!  Dang, it was so close!

>> 

>> It actually makes sense; was there any discussion about standardizing

>> this, with reasons to reject this construct, or is it just that we

>> aren't there yet?  I wouldn't be surprised if it were to appear in some

>> future version of C++.


> If what were to appear?


auto in explicit template arguments for template functions too.

> We only want 'auto' in places where it could be deduced, and there's

> no way in [temp.deduct.type] to deduce from explicit template

> arguments to a non-type template.


Well, it could be standardized in some way, e.g. autos could be deduced
from types of function arguments and expected return types, just like
function overloads, very much like typename template arguments are
currently deduced.  Having all explicit autos resolved would be a
requirement for a specialization to be viable.  Consider:

template <typename A, typename B, typename C> A foo(B b, C c);

...

  int x1 = foo("", 0); // ok, all args deduced
  int x2 = foo<int>("", 0); // ok, A is explicit, B and C are deduced

  int x3 = foo<int, auto, int>("", 0); // B could be deduced as in x2
  int x4 = foo<auto, auto, auto>("", 0); // all args deduced as in x1

  auto x5 = foo<auto>("", 0); // deduction of A from context fails

Cases in which auto appears in the top-level are the trivial ones, that
our type deduction machinery could support right away (it has to; it's
the same as deducing any typename argument left out).

More involved cases, in which auto appears other than at the top level,
could still be deduced in general, though we would need some more
implementation work to get them right:

  int *x6 = foo<auto *, auto       *, auto>("", 0);
  // deduced:   int     const char    int -> calls foo<int*,const char *,int>


I seems to me it would be a natural extension to the existing draft
specs; I guess the most common use would be to just tell the compiler to
figure out one specific template argument, when you only wish to
explicitly specify another after that: foo<auto, int>


>> Is this (in the patch below) the best spot to test for it?


> This seems like a plausible spot, but is there a reason not to check

> in cp_parser_template_id?


AFAICT we wouldn't always know, within cp_parser_template_id, whether
the id is a type or a function.  It seemed to me that we could
tentatively parse it as a type, and then reuse the same preparsed id as
a function.  If I were to check the argument list at that point,
assuming one or the other, we might reject something we should accept or
vice-versa, and we wouldn't revisit that decision.  I'm not saying I
tried and observed this, it is just a hunch based on observing how the
foo<auto>() expression was parsed and reparsed, and on extrapolations
from the preparsing of qualified names that I've recently had a close
encounter with ;-)

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill March 30, 2018, 2:47 p.m. | #5
On Thu, Mar 29, 2018 at 6:24 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Mar 28, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>> On Wed, Mar 28, 2018 at 5:06 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>> On Mar 23, 2018, Jason Merrill <jason@redhat.com> wrote:

>>>

>>>> On Fri, Mar 23, 2018 at 3:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>>>> +  /* Concepts allows 'auto' in template arguments, even multiple

>>>>> +     'auto's in a single argument.

>>>

>>>> I think that's only intended for class templates;

>>>

>>> Aah, that explains a lot!  Dang, it was so close!

>>>

>>> It actually makes sense; was there any discussion about standardizing

>>> this, with reasons to reject this construct, or is it just that we

>>> aren't there yet?  I wouldn't be surprised if it were to appear in some

>>> future version of C++.

>

>> If what were to appear?

>

> auto in explicit template arguments for template functions too.

>

>> We only want 'auto' in places where it could be deduced, and there's

>> no way in [temp.deduct.type] to deduce from explicit template

>> arguments to a non-type template.

>

> Well, it could be standardized in some way, e.g. autos could be deduced

> from types of function arguments and expected return types, just like

> function overloads, very much like typename template arguments are

> currently deduced.  Having all explicit autos resolved would be a

> requirement for a specialization to be viable.  Consider:

>

> template <typename A, typename B, typename C> A foo(B b, C c);

>

> ...

>

>   int x1 = foo("", 0); // ok, all args deduced

>   int x2 = foo<int>("", 0); // ok, A is explicit, B and C are deduced

>

>   int x3 = foo<int, auto, int>("", 0); // B could be deduced as in x2

>   int x4 = foo<auto, auto, auto>("", 0); // all args deduced as in x1

>

>   auto x5 = foo<auto>("", 0); // deduction of A from context fails

>

> Cases in which auto appears in the top-level are the trivial ones, that

> our type deduction machinery could support right away (it has to; it's

> the same as deducing any typename argument left out).

>

> More involved cases, in which auto appears other than at the top level,

> could still be deduced in general, though we would need some more

> implementation work to get them right:

>

>   int *x6 = foo<auto *, auto       *, auto>("", 0);

>   // deduced:   int     const char    int -> calls foo<int*,const char *,int>

>

>

> I seems to me it would be a natural extension to the existing draft

> specs; I guess the most common use would be to just tell the compiler to

> figure out one specific template argument, when you only wish to

> explicitly specify another after that: foo<auto, int>


I suppose that's possible.  Nobody has suggested such a thing to the
committee, however.

>>> Is this (in the patch below) the best spot to test for it?

>

>> This seems like a plausible spot, but is there a reason not to check

>> in cp_parser_template_id?

>

> AFAICT we wouldn't always know, within cp_parser_template_id, whether

> the id is a type or a function.


True, it looks like sometimes we build a TEMPLATE_ID_EXPR with an
IDENTIFIER_NODE.  Looking at tsubst_copy_and_build, I see that we
don't call finish_id_expression when substituting such a
TEMPLATE_ID_EXPR.  So maybe lookup_template_function and
lookup_template_variable are the right places for this test.

Jason
Alexandre Oliva March 31, 2018, 5 a.m. | #6
On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Thu, Mar 29, 2018 at 6:24 PM, Alexandre Oliva <aoliva@redhat.com> wrote:


>> AFAICT we wouldn't always know, within cp_parser_template_id, whether

>> the id is a type or a function.


> True, it looks like sometimes we build a TEMPLATE_ID_EXPR with an

> IDENTIFIER_NODE.  Looking at tsubst_copy_and_build, I see that we

> don't call finish_id_expression when substituting such a

> TEMPLATE_ID_EXPR.  So maybe lookup_template_function and

> lookup_template_variable are the right places for this test.


lookup_template_function is no good as it is, it's called twice with the
same (fns, arglist), from build_concept_check (within
cp_parser_maybe_partial_concept_id), and then directly from
cp_parser_template_id.  With the patchlet below, we get two copies of
the error message:

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 284eaf3cab66..b35f8076cfbd 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -8788,6 +8788,18 @@ lookup_template_function (tree fns, tree arglist)
       return error_mark_node;
     }
 
+  if (flag_concepts)
+    {
+      tree xauto = type_uses_auto (arglist);
+      if (xauto)
+	{
+	  error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (xauto)),
+		    "invalid use of %<auto%> in template argument"
+		    " for template function");
+	  return error_mark_node;
+	}
+    }
+
   if (BASELINK_P (fns))
     {
       BASELINK_FUNCTIONS (fns) = build2 (TEMPLATE_ID_EXPR,
@@ -9395,6 +9407,19 @@ lookup_template_variable (tree templ, tree arglist)
   if (flag_concepts && variable_concept_p (templ))
     /* Except that concepts are always bool.  */
     type = boolean_type_node;
+
+  if (flag_concepts)
+    {
+      tree xauto = type_uses_auto (arglist);
+      if (xauto)
+	{
+	  error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (xauto)),
+		    "invalid use of %<auto%> in template argument"
+		    " for template variable");
+	  return error_mark_node;
+	}
+    }
+
   return build2 (TEMPLATE_ID_EXPR, type, templ, arglist);
 }
 

So I tried the spot at cp_parser_template_id just after the spot that
tries a partial_concept_id, and then instead of two copies of the error
message, I got four:

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e946d0b72292..f93b3bb21307 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15856,6 +15856,13 @@ cp_parser_template_id (cp_parser *parser,
            && (template_id = (cp_parser_maybe_partial_concept_id
 			      (parser, templ, arguments))))
     return template_id;
+  else if (flag_concepts
+	   && (template_id = type_uses_auto (arguments)))
+    {
+      error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (template_id)),
+		"invalid use of %<auto%> in template argument");
+      return error_mark_node;
+    }
   else if (variable_template_p (templ))
     {
       template_id = lookup_template_variable (templ, arguments);


I guess this means we don't want to fail to parse the thing as what it
is and force the parser to try other things just because there's an auto
in the arglist.  Parsing it as what it is and then catching it later,
like I proposed before, seems to be working better.  It's not a parse
error, after all, it's rather a constraint on what otherwise legitimate
template arguments might contain.

Can you elaborate on any reasons to not proceed with the patch I
proposed?  (if the reason is the failure to call finish_id_expression
when tsubsting TEMPLATE_ID_EXPRs with identifiers in them, maybe we can
have the test there as well?)

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Alexandre Oliva March 31, 2018, 8:23 a.m. | #7
On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

> True, it looks like sometimes we build a TEMPLATE_ID_EXPR with an

> IDENTIFIER_NODE.  Looking at tsubst_copy_and_build, I see that we

> don't call finish_id_expression when substituting such a

> TEMPLATE_ID_EXPR.  So maybe lookup_template_function and

> lookup_template_variable are the right places for this test.


Nevermind the earlier email about multiple errors.  I realized that we
save the preparsed template_id right after the tests in
cp_parser_template_id, and if only I don't stop the rejected template
from being saved, we avoid the duplicate errors, as in the patch below.

A slight variant of this passed regstrap on i686- and x86_64-linux-gnu.
Ok to install, though it does not catch such cases as:

template <typename T>
void foo(T t) {
  typename T::template C<auto> u = t;
  T::template C<auto> (t);
  T::template C<auto>::f (t, u);
}

?


[PR c++/84979] reject auto in explicit tmpl args for tmpl-fn

With concepts, we accept auto in explicit template arguments, but we
should only accept them for template classes.  Passing them to
template functions is not allowed.  So, reject it.


for  gcc/cp/ChangeLog

	PR c++/84979
	* parser.c (cp_parser_check_for_auto_in_templ_arguments): New.
	(cp_parser_template_id): Use it to reject template args
	referencing auto for non-type templates.

for  gcc/testsuite/ChangeLog

	PR c++/84979
	* g++.dg/concepts/pr84979.C: New.
---
 gcc/cp/parser.c                         |   39 +++++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/concepts/pr84979.C |    9 +++++++
 2 files changed, 48 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e946d0b72292..c5ba2123ae96 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15690,6 +15690,42 @@ cp_parser_type_parameter (cp_parser* parser, bool *is_parameter_pack)
   return parameter;
 }
 
+/* If concepts are enabled and TEMPL does not identify a template
+   class, report occurrences of auto types in ARGUMENTS.  Return TRUE
+   if any such errors were reported.  */
+
+static bool
+cp_parser_check_for_auto_in_templ_arguments (cp_parser *parser ATTRIBUTE_UNUSED,
+					     tree templ,
+					     tree arguments)
+{
+  if (!flag_concepts)
+    return false;
+
+  if (identifier_p (templ)
+      || DECL_TYPE_TEMPLATE_P (templ)
+      || DECL_TEMPLATE_TEMPLATE_PARM_P (templ))
+    return false;
+
+  if (!arguments || TREE_CODE (arguments) != TREE_VEC)
+    return false;
+
+  bool errors = false;
+
+  for (int i = 0; i < TREE_VEC_LENGTH (arguments); i++)
+    {
+      tree xauto = type_uses_auto (TREE_VEC_ELT (arguments, i));
+      if (xauto)
+	{
+	  error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (xauto)),
+		    "invalid use of %<auto%> in template argument");
+	  errors = true;
+	}
+    }
+
+  return errors;
+}
+
 /* Parse a template-id.
 
    template-id:
@@ -15851,6 +15887,9 @@ cp_parser_template_id (cp_parser *parser,
       template_id
 	= finish_template_type (templ, arguments, entering_scope);
     }
+  else if (cp_parser_check_for_auto_in_templ_arguments (parser, templ,
+							arguments))
+    template_id = error_mark_node;
   /* A template-like identifier may be a partial concept id. */
   else if (flag_concepts
            && (template_id = (cp_parser_maybe_partial_concept_id
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979.C b/gcc/testsuite/g++.dg/concepts/pr84979.C
new file mode 100644
index 000000000000..c70b3756f2b8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename> void foo() {}
+
+void bar()
+{
+  foo<auto>(); // { dg-error "invalid|no match" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill April 3, 2018, 1:15 a.m. | #8
On Sat, Mar 31, 2018 at 4:23 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>> True, it looks like sometimes we build a TEMPLATE_ID_EXPR with an

>> IDENTIFIER_NODE.  Looking at tsubst_copy_and_build, I see that we

>> don't call finish_id_expression when substituting such a

>> TEMPLATE_ID_EXPR.  So maybe lookup_template_function and

>> lookup_template_variable are the right places for this test.

>

> Nevermind the earlier email about multiple errors.  I realized that we

> save the preparsed template_id right after the tests in

> cp_parser_template_id, and if only I don't stop the rejected template

> from being saved, we avoid the duplicate errors, as in the patch below.

>

> A slight variant of this passed regstrap on i686- and x86_64-linux-gnu.

> Ok to install, though it does not catch such cases as:

>

> template <typename T>

> void foo(T t) {

>   typename T::template C<auto> u = t;

>   T::template C<auto> (t);

>   T::template C<auto>::f (t, u);

> }

>

> ?


We should be able to distinguish those cases based on tag_type.

Jason
Alexandre Oliva April 3, 2018, 7:54 a.m. | #9
On Apr  2, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Sat, Mar 31, 2018 at 4:23 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

>> template <typename T>

>> void foo(T t) {

>>   typename T::template C<auto> u = t;

>>   T::template C<auto> (t);

>>   T::template C<auto>::f (t, u);

>> }


> We should be able to distinguish those cases based on tag_type.


Uhh...  I don't see how.

I mean, yeah, if we see typename or any other tag first, tag_type will
tell us we must find types for anything template-like, be it an
intermediate scope or the final typename.

However, if we see 'T::template C<auto> (t)', we can't tell whether T::C
is a template class and we're "calling a constructor" to create a
temporary out of t, or T::C is a static member template function that's
being called with t as an argument.  (ok, we know from the typename line
that C is a type, but nevermind that, it could be a different name in
the testcase, and then we wouldn't; heck, couldn't it even be a template
function hiding the template class of the same name?)

And then, while we're parsing "template C<auto>", we haven't yet reached
the '::' after the closing angle bracket that would tell us to regard
the id necessarily as a typename, so I don't see how we'd get a
scope_type in tag_type for the third case.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill April 3, 2018, 3:46 p.m. | #10
On Tue, Apr 3, 2018 at 3:54 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Apr  2, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>> On Sat, Mar 31, 2018 at 4:23 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

>>> template <typename T>

>>> void foo(T t) {

>>>   typename T::template C<auto> u = t;

>>>   T::template C<auto> (t);

>>>   T::template C<auto>::f (t, u);

>>> }

>

>> We should be able to distinguish those cases based on tag_type.


>[...]

> And then, while we're parsing "template C<auto>", we haven't yet reached

> the '::' after the closing angle bracket that would tell us to regard

> the id necessarily as a typename, so I don't see how we'd get a

> scope_type in tag_type for the third case.


Ah, good point.  Then perhaps put the new function in pt.c and also
call it from tsubst_copy_and_build?

Jason
Alexandre Oliva April 4, 2018, 2:38 a.m. | #11
On Apr  3, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Tue, Apr 3, 2018 at 3:54 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>> On Apr  2, 2018, Jason Merrill <jason@redhat.com> wrote:

>> 

>>> On Sat, Mar 31, 2018 at 4:23 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>>> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

>>>> template <typename T>

>>>> void foo(T t) {

>>>> typename T::template C<auto> u = t;

>>>> T::template C<auto> (t);

>>>> T::template C<auto>::f (t, u);

>>>> }

>> 

>>> We should be able to distinguish those cases based on tag_type.


>> [...]

>> And then, while we're parsing "template C<auto>", we haven't yet reached

>> the '::' after the closing angle bracket that would tell us to regard

>> the id necessarily as a typename, so I don't see how we'd get a

>> scope_type in tag_type for the third case.


> Ah, good point.  Then perhaps put the new function in pt.c and also

> call it from tsubst_copy_and_build?


I couldn't figure out how to trigger checks in tsubst_copy_and_build;
the testcases I threw at it all went through tsubst_qualified_id.  I
think we can only get an identifier_p in a TEMPLATE_ID_EXPR when it's a
qualified id naming a member of another template; unqualified names that
could possibly take explicit template arguments would have to have
visible declarations, and then we'd get (an overload of) the
declarations in the TEMPLATE_ID_EXPR.  I see evidence in pt.c that some
cases of Koenig lookup may involve identifier_p template substitution,
and I see evidence in the standard that the unqualified template-id
could have explicit template args, but...  we won't regard <whatever> as
a template arg list if it's not preceded by an id that names a template,
be it a template-dependent qualified id preceded by the 'template'
keyword, be a template found by name lookup.  If we find an unrelated
template function (as in the snippet below), we'll stick to it.  So it
seems to me that the patch below is all we need.  Am I missing anything?

struct foo { template <typename T> friend void bar(T& t); };
template <int> void bar();
template <typename T> void f(T& x) { bar<auto> (x); }
void g(foo& x) { f (x); }


Regstrapping on i686- and x86_64-linux-gnu.  Ok to install if it passes?


[PR c++/84979] reject auto in explicit tmpl args for tmpl-fn

With concepts, we accept auto in explicit template arguments, but we
should only accept them for template classes.  Passing them to
template functions or variables is not allowed.  So, reject it, at
parse time if possible, at specialization time otherwise.


for  gcc/cp/ChangeLog

	PR c++/84979
	* pt.c (check_auto_in_tmpl_args): New.
	(tsubst_qualified_id): Use it to reject template args
	referencing auto for non-type templates.
	* parser.c (cp_parser_template_id): Likewise.
	* cp-tree.h (check_auto_in_tmpl_args): Declare.
	* typeck2.c (build_functional_cast): Report correct location
	for invalid use of auto.

for  gcc/testsuite/ChangeLog

	PR c++/84979
	* g++.dg/concepts/pr84979.C: New.
	* g++.dg/concepts/pr84979-2.C: New.
	* g++.dg/concepts/pr84979-3.C: New.
---
 gcc/cp/cp-tree.h                          |    1 +
 gcc/cp/parser.c                           |   10 +++++-
 gcc/cp/pt.c                               |   52 +++++++++++++++++++++++++++++
 gcc/cp/typeck2.c                          |    3 +-
 gcc/testsuite/g++.dg/concepts/pr84979-2.C |   41 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/concepts/pr84979-3.C |   45 +++++++++++++++++++++++++
 gcc/testsuite/g++.dg/concepts/pr84979.C   |    9 +++++
 7 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-2.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-3.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index db79338035da..4777aa34cdbe 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6496,6 +6496,7 @@ extern void maybe_show_extern_c_location (void);
 
 /* in pt.c */
 extern bool check_template_shadow		(tree);
+extern bool check_auto_in_tmpl_args             (tree, tree);
 extern tree get_innermost_template_args		(tree, int);
 extern void maybe_begin_member_template_processing (tree);
 extern void maybe_end_member_template_processing (void);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e946d0b72292..40a1e5549f02 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15831,8 +15831,16 @@ cp_parser_template_id (cp_parser *parser,
   location_t combined_loc
     = make_location (token->location, token->location, finish_loc);
 
+  /* Check for concepts autos where they don't belong.  We could
+     identify types in some cases of idnetifier TEMPL, looking ahead
+     for a CPP_SCOPE, but that would buy us nothing: we accept auto in
+     types.  We reject them in functions, but if what we have is an
+     identifier, even with none_type we can't conclude it's NOT a
+     type, we have to wait for template substitution.  */
+  if (flag_concepts && check_auto_in_tmpl_args (templ, arguments))
+    template_id = error_mark_node;
   /* Build a representation of the specialization.  */
-  if (identifier_p (templ))
+  else if (identifier_p (templ))
     template_id = build_min_nt_loc (combined_loc,
 				    TEMPLATE_ID_EXPR,
 				    templ, arguments);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 284eaf3cab66..a5c489fc0768 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14890,6 +14890,15 @@ tsubst_qualified_id (tree qualified_id, tree args,
 						    complain);
       else
 	expr = lookup_template_function (expr, template_args);
+
+      /* We may be repeating a check already done during parsing, but
+	 if it was well-formed and passed then, it will pass again
+	 now, and if it didn't, we wouldn't have got here.  The case
+	 we want to catch is when we couldn't tell then, and can now,
+	 namely when templ prior to substitution was an
+	 identifier.  */
+      if (flag_concepts && check_auto_in_tmpl_args (expr, template_args))
+	return error_mark_node;
     }
 
   if (expr == error_mark_node && complain & tf_error)
@@ -26500,6 +26509,49 @@ type_uses_auto (tree type)
     return find_type_usage (type, is_auto);
 }
 
+/* Report ill-formed occurrences of auto types in ARGUMENTS.  If
+   concepts are enabled, auto is acceptable in template arguments, but
+   only when TEMPL identifies a template class.  Return TRUE if any
+   such errors were reported.  */
+
+bool
+check_auto_in_tmpl_args (tree tmpl, tree args)
+{
+  /* If there were previous errors, nevermind.  */
+  if (!args || TREE_CODE (args) != TREE_VEC)
+    return false;
+
+  /* If TMPL is an identifier, we're parsing and we can't tell yet
+     whether TMPL is supposed to be a type, a function or a variable.
+     We'll only be able to tell during template substitution, so we
+     expect to be called again then.  If concepts are enabled and we
+     know we have a type, we're ok.  */
+  if (flag_concepts
+      && (identifier_p (tmpl)
+	  || (DECL_P (tmpl)
+	      &&  (DECL_TYPE_TEMPLATE_P (tmpl)
+		   || DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl)))))
+    return false;
+
+  /* Quickly search for any occurrences of auto; usually there won't
+     be any, and then we'll avoid allocating the vector.  */
+  if (!type_uses_auto (args))
+    return false;
+
+  bool errors = false;
+
+  tree vec = extract_autos (args);
+  for (int i = 0; i < TREE_VEC_LENGTH (vec); i++)
+    {
+      tree xauto = TREE_VALUE (TREE_VEC_ELT (vec, i));
+      error_at (DECL_SOURCE_LOCATION (xauto),
+		"invalid use of %qT in template argument", xauto);
+      errors = true;
+    }
+
+  return errors;
+}
+
 /* For a given template T, return the vector of typedefs referenced
    in T for which access check is needed at T instantiation time.
    T is either  a FUNCTION_DECL or a RECORD_TYPE.
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 3aae0a362d5d..3bdeae1501f5 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -2081,7 +2081,8 @@ build_functional_cast (tree exp, tree parms, tsubst_flags_t complain)
       if (!CLASS_PLACEHOLDER_TEMPLATE (anode))
 	{
 	  if (complain & tf_error)
-	    error ("invalid use of %qT", anode);
+	    error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (anode)),
+		      "invalid use of %qT", anode);
 	  return error_mark_node;
 	}
       else if (!parms)
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-2.C b/gcc/testsuite/g++.dg/concepts/pr84979-2.C
new file mode 100644
index 000000000000..ce69a0f8ac53
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-2.C
@@ -0,0 +1,41 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template <typename T>
+void foo1(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  (typename T::template D<auto> (t)); // { dg-error "invalid" }
+}
+
+struct T1 {
+  template <typename T> struct C {
+    C(T1&);
+    static void f(T1&, C&);
+  };
+  template <typename T> struct D {
+    D(T1&);
+  };
+};
+
+template <typename T>
+void foo2(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  T::template D<auto> (t); // { dg-error "invalid" }
+}
+
+struct T2 {
+  template <typename T> struct C {
+    C(T2&);
+    static void f(T2&, C&);
+  };
+  template <typename T> static void D(T2&);
+};
+
+void f(T1& t1, T2& t2) {
+  foo1 (t1);
+  foo2 (t2);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-3.C b/gcc/testsuite/g++.dg/concepts/pr84979-3.C
new file mode 100644
index 000000000000..3809c2d3033b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-3.C
@@ -0,0 +1,45 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+// This is like pr84979-2.C, except that we swap the types passed to
+// the template functions foo1 and foo2, so that the expectations WRT
+// D's typeness are not met.
+
+template <typename T>
+void foo1(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  (typename T::template D<auto> (t)); // { dg-error "invalid" }
+}
+
+struct T1 {
+  template <typename T> struct C {
+    C(T1&);
+    static void f(T1&, C&);
+  };
+  template <typename T> struct D {
+    D(T1&);
+  };
+};
+
+template <typename T>
+void foo2(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  T::template D<auto> (t); // { dg-error "yields a type" }
+}
+
+struct T2 {
+  template <typename T> struct C {
+    C(T2&);
+    static void f(T2&, C&);
+  };
+  template <typename T> static void D(T2&);
+};
+
+void f(T1& t1, T2& t2) {
+  foo1 (t2);
+  foo2 (t1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979.C b/gcc/testsuite/g++.dg/concepts/pr84979.C
new file mode 100644
index 000000000000..9bd40df103a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename> void foo() {}
+
+void bar()
+{
+  foo<auto>(); // { dg-error "invalid" }
+}



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill April 4, 2018, 2:34 p.m. | #12
On Tue, Apr 3, 2018 at 10:38 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Apr  3, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>> On Tue, Apr 3, 2018 at 3:54 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>> On Apr  2, 2018, Jason Merrill <jason@redhat.com> wrote:

>>>

>>>> On Sat, Mar 31, 2018 at 4:23 AM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>>>> On Mar 30, 2018, Jason Merrill <jason@redhat.com> wrote:

>>>>> template <typename T>

>>>>> void foo(T t) {

>>>>> typename T::template C<auto> u = t;

>>>>> T::template C<auto> (t);

>>>>> T::template C<auto>::f (t, u);

>>>>> }

>>>

>>>> We should be able to distinguish those cases based on tag_type.

>

>>> [...]

>>> And then, while we're parsing "template C<auto>", we haven't yet reached

>>> the '::' after the closing angle bracket that would tell us to regard

>>> the id necessarily as a typename, so I don't see how we'd get a

>>> scope_type in tag_type for the third case.

>

>> Ah, good point.  Then perhaps put the new function in pt.c and also

>> call it from tsubst_copy_and_build?

>

> I couldn't figure out how to trigger checks in tsubst_copy_and_build;

> the testcases I threw at it all went through tsubst_qualified_id.  I

> think we can only get an identifier_p in a TEMPLATE_ID_EXPR when it's a

> qualified id naming a member of another template; unqualified names that

> could possibly take explicit template arguments would have to have

> visible declarations, and then we'd get (an overload of) the

> declarations in the TEMPLATE_ID_EXPR.  I see evidence in pt.c that some

> cases of Koenig lookup may involve identifier_p template substitution,

> and I see evidence in the standard that the unqualified template-id

> could have explicit template args, but...  we won't regard <whatever> as

> a template arg list if it's not preceded by an id that names a template,

> be it a template-dependent qualified id preceded by the 'template'

> keyword, be a template found by name lookup.  If we find an unrelated

> template function (as in the snippet below), we'll stick to it.  So it

> seems to me that the patch below is all we need.  Am I missing anything?

>

> struct foo { template <typename T> friend void bar(T& t); };

> template <int> void bar();

> template <typename T> void f(T& x) { bar<auto> (x); }

> void g(foo& x) { f (x); }

>

>

> Regstrapping on i686- and x86_64-linux-gnu.  Ok to install if it passes?

>

>

> [PR c++/84979] reject auto in explicit tmpl args for tmpl-fn

>

> With concepts, we accept auto in explicit template arguments, but we

> should only accept them for template classes.  Passing them to

> template functions or variables is not allowed.  So, reject it, at

> parse time if possible, at specialization time otherwise.

>

>

> for  gcc/cp/ChangeLog

>

>         PR c++/84979

>         * pt.c (check_auto_in_tmpl_args): New.

>         (tsubst_qualified_id): Use it to reject template args

>         referencing auto for non-type templates.

>         * parser.c (cp_parser_template_id): Likewise.

>         * cp-tree.h (check_auto_in_tmpl_args): Declare.

>         * typeck2.c (build_functional_cast): Report correct location

>         for invalid use of auto.

>

> for  gcc/testsuite/ChangeLog

>

>         PR c++/84979

>         * g++.dg/concepts/pr84979.C: New.

>         * g++.dg/concepts/pr84979-2.C: New.

>         * g++.dg/concepts/pr84979-3.C: New.

> ---

>  gcc/cp/cp-tree.h                          |    1 +

>  gcc/cp/parser.c                           |   10 +++++-

>  gcc/cp/pt.c                               |   52 +++++++++++++++++++++++++++++

>  gcc/cp/typeck2.c                          |    3 +-

>  gcc/testsuite/g++.dg/concepts/pr84979-2.C |   41 +++++++++++++++++++++++

>  gcc/testsuite/g++.dg/concepts/pr84979-3.C |   45 +++++++++++++++++++++++++

>  gcc/testsuite/g++.dg/concepts/pr84979.C   |    9 +++++

>  7 files changed, 159 insertions(+), 2 deletions(-)

>  create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-2.C

>  create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-3.C

>  create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C

>

> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h

> index db79338035da..4777aa34cdbe 100644

> --- a/gcc/cp/cp-tree.h

> +++ b/gcc/cp/cp-tree.h

> @@ -6496,6 +6496,7 @@ extern void maybe_show_extern_c_location (void);

>

>  /* in pt.c */

>  extern bool check_template_shadow              (tree);

> +extern bool check_auto_in_tmpl_args             (tree, tree);

>  extern tree get_innermost_template_args                (tree, int);

>  extern void maybe_begin_member_template_processing (tree);

>  extern void maybe_end_member_template_processing (void);

> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> index e946d0b72292..40a1e5549f02 100644

> --- a/gcc/cp/parser.c

> +++ b/gcc/cp/parser.c

> @@ -15831,8 +15831,16 @@ cp_parser_template_id (cp_parser *parser,

>    location_t combined_loc

>      = make_location (token->location, token->location, finish_loc);

>

> +  /* Check for concepts autos where they don't belong.  We could

> +     identify types in some cases of idnetifier TEMPL, looking ahead

> +     for a CPP_SCOPE, but that would buy us nothing: we accept auto in

> +     types.  We reject them in functions, but if what we have is an

> +     identifier, even with none_type we can't conclude it's NOT a

> +     type, we have to wait for template substitution.  */

> +  if (flag_concepts && check_auto_in_tmpl_args (templ, arguments))

> +    template_id = error_mark_node;

>    /* Build a representation of the specialization.  */

> -  if (identifier_p (templ))

> +  else if (identifier_p (templ))

>      template_id = build_min_nt_loc (combined_loc,

>                                     TEMPLATE_ID_EXPR,

>                                     templ, arguments);

> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

> index 284eaf3cab66..a5c489fc0768 100644

> --- a/gcc/cp/pt.c

> +++ b/gcc/cp/pt.c

> @@ -14890,6 +14890,15 @@ tsubst_qualified_id (tree qualified_id, tree args,

>                                                     complain);

>        else

>         expr = lookup_template_function (expr, template_args);

> +

> +      /* We may be repeating a check already done during parsing, but

> +        if it was well-formed and passed then, it will pass again

> +        now, and if it didn't, we wouldn't have got here.  The case

> +        we want to catch is when we couldn't tell then, and can now,

> +        namely when templ prior to substitution was an

> +        identifier.  */

> +      if (flag_concepts && check_auto_in_tmpl_args (expr, template_args))

> +       return error_mark_node;


I think we want this added code to go at the beginning of the block,
before we've wrapped the template in a TEMPLATE_ID_EXPR.  OK with that
change.

Jason
Alexandre Oliva April 4, 2018, 4:40 p.m. | #13
On Apr  4, 2018, Jason Merrill <jason@redhat.com> wrote:

>> else

>> expr = lookup_template_function (expr, template_args);

>> +

>> +      /* We may be repeating a check already done during parsing, but

>> +        if it was well-formed and passed then, it will pass again

>> +        now, and if it didn't, we wouldn't have got here.  The case

>> +        we want to catch is when we couldn't tell then, and can now,

>> +        namely when templ prior to substitution was an

>> +        identifier.  */

>> +      if (flag_concepts && check_auto_in_tmpl_args (expr, template_args))

>> +       return error_mark_node;


> I think we want this added code to go at the beginning of the block,


Uhh, it can't be too early, or we will still have an identifier, and
then the check function will just return.

> before we've wrapped the template in a TEMPLATE_ID_EXPR.  OK with that

> change.


Aah, maybe you mean the smaller block under 'if (is_template)', as in
the so-modified patch below?  Heh, yeah, lookup_template_function and
lookup_template_variable are very misleading function names!  They don't
perform any of the lookups I expected of them! ;-)

I'm testing this patch now, and if it passes I'll proceed to install it
under the assumption that this is what you approved.

Thanks!


[PR c++/84979] reject auto in explicit tmpl args for tmpl-fn

With concepts, we accept auto in explicit template arguments, but we
should only accept them for template classes.  Passing them to
template functions or variables is not allowed.  So, reject it, at
parse time if possible, at specialization time otherwise.


for  gcc/cp/ChangeLog

	PR c++/84979
	* pt.c (check_auto_in_tmpl_args): New.
	(tsubst_qualified_id): Use it to reject template args
	referencing auto for non-type templates.
	* parser.c (cp_parser_template_id): Likewise.
	* cp-tree.h (check_auto_in_tmpl_args): Declare.
	* typeck2.c (build_functional_cast): Report correct location
	for invalid use of auto.

for  gcc/testsuite/ChangeLog

	PR c++/84979
	* g++.dg/concepts/pr84979.C: New.
	* g++.dg/concepts/pr84979-2.C: New.
	* g++.dg/concepts/pr84979-3.C: New.
---
 gcc/cp/cp-tree.h                          |    1 +
 gcc/cp/parser.c                           |   10 +++++-
 gcc/cp/pt.c                               |   52 +++++++++++++++++++++++++++++
 gcc/cp/typeck2.c                          |    3 +-
 gcc/testsuite/g++.dg/concepts/pr84979-2.C |   41 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/concepts/pr84979-3.C |   45 +++++++++++++++++++++++++
 gcc/testsuite/g++.dg/concepts/pr84979.C   |    9 +++++
 7 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-2.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-3.C
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f7bacd08c8f8..c8f2b1696525 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6500,6 +6500,7 @@ extern void maybe_show_extern_c_location (void);
 
 /* in pt.c */
 extern bool check_template_shadow		(tree);
+extern bool check_auto_in_tmpl_args             (tree, tree);
 extern tree get_innermost_template_args		(tree, int);
 extern void maybe_begin_member_template_processing (tree);
 extern void maybe_end_member_template_processing (void);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d526a4eb3652..af19f77acca3 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15831,8 +15831,16 @@ cp_parser_template_id (cp_parser *parser,
   location_t combined_loc
     = make_location (token->location, token->location, finish_loc);
 
+  /* Check for concepts autos where they don't belong.  We could
+     identify types in some cases of idnetifier TEMPL, looking ahead
+     for a CPP_SCOPE, but that would buy us nothing: we accept auto in
+     types.  We reject them in functions, but if what we have is an
+     identifier, even with none_type we can't conclude it's NOT a
+     type, we have to wait for template substitution.  */
+  if (flag_concepts && check_auto_in_tmpl_args (templ, arguments))
+    template_id = error_mark_node;
   /* Build a representation of the specialization.  */
-  if (identifier_p (templ))
+  else if (identifier_p (templ))
     template_id = build_min_nt_loc (combined_loc,
 				    TEMPLATE_ID_EXPR,
 				    templ, arguments);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 80670a4826b6..4bc1f6e61e46 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14903,6 +14903,15 @@ tsubst_qualified_id (tree qualified_id, tree args,
 
   if (is_template)
     {
+      /* We may be repeating a check already done during parsing, but
+	 if it was well-formed and passed then, it will pass again
+	 now, and if it didn't, we wouldn't have got here.  The case
+	 we want to catch is when we couldn't tell then, and can now,
+	 namely when templ prior to substitution was an
+	 identifier.  */
+      if (flag_concepts && check_auto_in_tmpl_args (expr, template_args))
+	return error_mark_node;
+
       if (variable_template_p (expr))
 	expr = lookup_and_finish_template_variable (expr, template_args,
 						    complain);
@@ -26547,6 +26556,49 @@ type_uses_auto (tree type)
     return find_type_usage (type, is_auto);
 }
 
+/* Report ill-formed occurrences of auto types in ARGUMENTS.  If
+   concepts are enabled, auto is acceptable in template arguments, but
+   only when TEMPL identifies a template class.  Return TRUE if any
+   such errors were reported.  */
+
+bool
+check_auto_in_tmpl_args (tree tmpl, tree args)
+{
+  /* If there were previous errors, nevermind.  */
+  if (!args || TREE_CODE (args) != TREE_VEC)
+    return false;
+
+  /* If TMPL is an identifier, we're parsing and we can't tell yet
+     whether TMPL is supposed to be a type, a function or a variable.
+     We'll only be able to tell during template substitution, so we
+     expect to be called again then.  If concepts are enabled and we
+     know we have a type, we're ok.  */
+  if (flag_concepts
+      && (identifier_p (tmpl)
+	  || (DECL_P (tmpl)
+	      &&  (DECL_TYPE_TEMPLATE_P (tmpl)
+		   || DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl)))))
+    return false;
+
+  /* Quickly search for any occurrences of auto; usually there won't
+     be any, and then we'll avoid allocating the vector.  */
+  if (!type_uses_auto (args))
+    return false;
+
+  bool errors = false;
+
+  tree vec = extract_autos (args);
+  for (int i = 0; i < TREE_VEC_LENGTH (vec); i++)
+    {
+      tree xauto = TREE_VALUE (TREE_VEC_ELT (vec, i));
+      error_at (DECL_SOURCE_LOCATION (xauto),
+		"invalid use of %qT in template argument", xauto);
+      errors = true;
+    }
+
+  return errors;
+}
+
 /* For a given template T, return the vector of typedefs referenced
    in T for which access check is needed at T instantiation time.
    T is either  a FUNCTION_DECL or a RECORD_TYPE.
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 3aae0a362d5d..3bdeae1501f5 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -2081,7 +2081,8 @@ build_functional_cast (tree exp, tree parms, tsubst_flags_t complain)
       if (!CLASS_PLACEHOLDER_TEMPLATE (anode))
 	{
 	  if (complain & tf_error)
-	    error ("invalid use of %qT", anode);
+	    error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (anode)),
+		      "invalid use of %qT", anode);
 	  return error_mark_node;
 	}
       else if (!parms)
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-2.C b/gcc/testsuite/g++.dg/concepts/pr84979-2.C
new file mode 100644
index 000000000000..ce69a0f8ac53
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-2.C
@@ -0,0 +1,41 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template <typename T>
+void foo1(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  (typename T::template D<auto> (t)); // { dg-error "invalid" }
+}
+
+struct T1 {
+  template <typename T> struct C {
+    C(T1&);
+    static void f(T1&, C&);
+  };
+  template <typename T> struct D {
+    D(T1&);
+  };
+};
+
+template <typename T>
+void foo2(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  T::template D<auto> (t); // { dg-error "invalid" }
+}
+
+struct T2 {
+  template <typename T> struct C {
+    C(T2&);
+    static void f(T2&, C&);
+  };
+  template <typename T> static void D(T2&);
+};
+
+void f(T1& t1, T2& t2) {
+  foo1 (t1);
+  foo2 (t2);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-3.C b/gcc/testsuite/g++.dg/concepts/pr84979-3.C
new file mode 100644
index 000000000000..3809c2d3033b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-3.C
@@ -0,0 +1,45 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+// This is like pr84979-2.C, except that we swap the types passed to
+// the template functions foo1 and foo2, so that the expectations WRT
+// D's typeness are not met.
+
+template <typename T>
+void foo1(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  (typename T::template D<auto> (t)); // { dg-error "invalid" }
+}
+
+struct T1 {
+  template <typename T> struct C {
+    C(T1&);
+    static void f(T1&, C&);
+  };
+  template <typename T> struct D {
+    D(T1&);
+  };
+};
+
+template <typename T>
+void foo2(T& t) {
+  typename T::template C<void> tcv = t;
+  typename T::template C<auto> u = tcv;
+  T::template C<auto>::f (tcv, u); // { dg-error "incomplete" }
+  T::template D<auto> (t); // { dg-error "yields a type" }
+}
+
+struct T2 {
+  template <typename T> struct C {
+    C(T2&);
+    static void f(T2&, C&);
+  };
+  template <typename T> static void D(T2&);
+};
+
+void f(T1& t1, T2& t2) {
+  foo1 (t2);
+  foo2 (t1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979.C b/gcc/testsuite/g++.dg/concepts/pr84979.C
new file mode 100644
index 000000000000..9bd40df103a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename> void foo() {}
+
+void bar()
+{
+  foo<auto>(); // { dg-error "invalid" }
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 5293c2b5491b..2b9e5f1ada45 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19109,7 +19109,10 @@  fn_type_unification (tree fn,
               parameter_pack = TEMPLATE_PARM_PARAMETER_PACK (parm);
             }
 
-	  if (!parameter_pack && targ == NULL_TREE)
+	  if (!parameter_pack
+	      && (targ == NULL_TREE
+		  || (flag_concepts
+		      && type_uses_auto (targ))))
 	    /* No explicit argument for this template parameter.  */
 	    incomplete = true;
 
@@ -19891,7 +19894,12 @@  type_unification_real (tree tparms,
               ARGUMENT_PACK_EXPLICIT_ARGS (targ) = NULL_TREE;
             }
 
-	  if (targ || tparm == error_mark_node)
+	  if ((targ
+	       && (!flag_concepts
+		   /* We have to unify explicitly-passed template args
+		      involving auto types.  */
+		   || !type_uses_auto (targ)))
+	      || tparm == error_mark_node)
 	    continue;
 	  tparm = TREE_VALUE (tparm);
 
@@ -20952,6 +20960,37 @@  unify (tree tparms, tree targs, tree parm, tree arg, int strict,
   if (parm == any_targ_node || arg == any_targ_node)
     return unify_success (explain_p);
 
+  /* Concepts allows 'auto' in template arguments, even multiple
+     'auto's in a single argument.  We don't know how to deal with
+     them here: see how do_auto_deduction uses extract_autos to
+     construct a separate template args vec, that such autos index.
+     Since we don't do any of that, we can only substitute toplevel
+     auto parameters, and only when their index happens to match their
+     position in the template argument list.  Catch deviations instead
+     of corrupting targs.  */
+  if (flag_concepts && is_auto (parm)
+      && ((TEMPLATE_TYPE_IDX (parm)
+	   >= TREE_VEC_LENGTH (INNERMOST_TEMPLATE_ARGS (targs)))
+	  || (TREE_CODE (TREE_VALUE (TREE_VEC_ELT
+				     (tparms, TEMPLATE_TYPE_IDX (parm))))
+	      != TYPE_DECL)
+	  || (TREE_VEC_ELT (INNERMOST_TEMPLATE_ARGS (targs),
+			    TEMPLATE_TYPE_IDX (parm)) != parm
+	      /* do_auto_deduction calls us properly, i.e., with the
+		 extra targs level, so don't complain about its uses.
+		 ??? Is there a better/surer way to test for the
+		 proper uses?  */
+	      && TREE_VEC_ELT (INNERMOST_TEMPLATE_ARGS (targs),
+			       TEMPLATE_TYPE_IDX (parm)) != NULL_TREE
+	      && type_uses_auto (TREE_VEC_ELT
+				 (INNERMOST_TEMPLATE_ARGS (targs),
+				  TEMPLATE_TYPE_IDX (parm))))))
+    {
+      if (complain & tf_error)
+	sorry ("unsupported use of auto in explicit template parameter list");
+      return unify_invalid (explain_p);
+    }
+
   /* If PARM uses template parameters, then we can't bail out here,
      even if ARG == PARM, since we won't record unifications for the
      template parameters.  We might need them if we're trying to
@@ -21147,8 +21186,13 @@  unify (tree tparms, tree targs, tree parm, tree arg, int strict,
 	{
 	  /* Deduce template name TT from TT, TT<>, TT<T> and TT<i>.  */
 
+	  /* Substitute (some) concepts-specified auto parms.
+	     ??? Can autos ever pass the test above?  */
+	  if (targ != NULL_TREE && flag_concepts && parm == targ
+	      && is_auto (parm))
+	    ;
 	  /* Simple cases: Value already set, does match or doesn't.  */
-	  if (targ != NULL_TREE && template_args_equal (targ, arg))
+	  else if (targ != NULL_TREE && template_args_equal (targ, arg))
 	    return unify_success (explain_p);
 	  else if (targ)
 	    return unify_inconsistency (explain_p, parm, targ, arg);
@@ -21170,8 +21214,12 @@  unify (tree tparms, tree targs, tree parm, tree arg, int strict,
 	  if (arg == error_mark_node)
 	    return unify_invalid (explain_p);
 
+	  /* Substitute (some) concepts-specified auto parms.  */
+	  if (targ != NULL_TREE && flag_concepts && parm == targ
+	      && is_auto (parm))
+	    ;
 	  /* Simple cases: Value already set, does match or doesn't.  */
-	  if (targ != NULL_TREE && same_type_p (targ, arg))
+	  else if (targ != NULL_TREE && same_type_p (targ, arg))
 	    return unify_success (explain_p);
 	  else if (targ)
 	    return unify_inconsistency (explain_p, parm, targ, arg);
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-2.C b/gcc/testsuite/g++.dg/concepts/pr84979-2.C
new file mode 100644
index 000000000000..140b051b311e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-2.C
@@ -0,0 +1,9 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename T, int> void foo(T t) {}
+
+void bar()
+{
+  foo<auto, 0>(bar);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-3.C b/gcc/testsuite/g++.dg/concepts/pr84979-3.C
new file mode 100644
index 000000000000..93a0140e8390
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-3.C
@@ -0,0 +1,9 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename T, int> void foo(T t) {}
+
+void bar()
+{
+  foo<auto (*)(), 0>(bar); // { dg-bogus "unimplemented|no matching function" "" { xfail *-*-* } }
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979-4.C b/gcc/testsuite/g++.dg/concepts/pr84979-4.C
new file mode 100644
index 000000000000..08054d0d832a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979-4.C
@@ -0,0 +1,9 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<int, typename T> void foo(T t) {}
+
+void bar()
+{
+  foo<0, auto>(bar); // { dg-bogus "unimplemented|no matching function" "" { xfail *-*-* } }
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr84979.C b/gcc/testsuite/g++.dg/concepts/pr84979.C
new file mode 100644
index 000000000000..c70b3756f2b8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr84979.C
@@ -0,0 +1,9 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename> void foo() {}
+
+void bar()
+{
+  foo<auto>(); // { dg-error "invalid|no match" }
+}