[C++ Patch] Improve grokdeclarator error

[C++ Patch] Improve grokdeclarator error

[Paolo Carlini]

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

Hi,

over the last weeks, while working on various batches of location 
improvements (a new one is forthcoming, in case you are wondering ;) I 
noticed this grokdeclarator diagnostic, one of those not exercised by 
our testsuite. While constructing a testcase I realized that probably 
it's better to immediately return error_mark_node, and avoid an 
additional redundant error about variable or field declared void or even 
about "fancy" variable templates, depending on the return type of the 
function template. Tested x86_64-linux.

Thanks, Paolo.

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


[-- Attachment #2: CL_locs_50 --]
[-- Type: text/plain, Size: 298 bytes --]

/cp
2019-09-26  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (grokdeclarator): Immediately return error_mark_node
	upon error about template-id used as a declarator.

/testsuite
2019-09-26  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/diagnostic/template-id-as-declarator-1.C: New.

[-- Attachment #3: patch_locs_50 --]
[-- Type: text/plain, Size: 1120 bytes --]

Index: testsuite/g++.dg/diagnostic/template-id-as-declarator-1.C
===================================================================
--- testsuite/g++.dg/diagnostic/template-id-as-declarator-1.C	(nonexistent)
+++ testsuite/g++.dg/diagnostic/template-id-as-declarator-1.C	(working copy)
@@ -0,0 +1,5 @@
+template<typename>
+void foo() {}
+
+template void foo<int>(  // { dg-error "15:template-id .foo<int>. used as a declarator" }
+// { dg-error "expected" "" { target *-*-* } .-1 }
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 276151)
+++ cp/decl.c	(working copy)
@@ -12078,9 +12078,9 @@ grokdeclarator (const cp_declarator *declarator,
       && !FUNC_OR_METHOD_TYPE_P (type)
       && !variable_template_p (TREE_OPERAND (unqualified_id, 0)))
     {
-      error ("template-id %qD used as a declarator",
-	     unqualified_id);
-      unqualified_id = dname;
+      error_at (id_loc, "template-id %qD used as a declarator",
+		unqualified_id);
+      return error_mark_node;
     }
 
   /* If TYPE is a FUNCTION_TYPE, but the function name was explicitly

Re: [C++ Patch] Improve grokdeclarator error

[Jason Merrill]

On 9/26/19 3:39 PM, Paolo Carlini wrote:
> +template void foo<int>(  // { dg-error "15:template-id .foo<int>. used as a declarator" }

That's a strange diagnostic message; there's nothing wrong with using a 
template-id as a declarator.  Why are we even calling grokdeclarator 
when we hit EOF in the middle of the declarator?

Jason

Re: [C++ Patch] Improve grokdeclarator error

[Paolo Carlini]

Hi,

On 28/09/19 04:05, Jason Merrill wrote:
> On 9/26/19 3:39 PM, Paolo Carlini wrote:
>> +template void foo<int>(  // { dg-error "15:template-id .foo<int>. 
>> used as a declarator" }
>
> That's a strange diagnostic message; there's nothing wrong with using 
> a template-id as a declarator.  Why are we even calling grokdeclarator 
> when we hit EOF in the middle of the declarator?

It's indeed a weird situation. Note, by the way, that for

template void foo<int>;

we end up giving the same diagnostic, only, the location was already fine.

Anyway, to explain why I say weird, clang believes it's dealing with an 
explicit instantiation:

explicit instantiation of 'foo' does not refer to a function template, 
variable template, member function, member class, or static data member

whereas EDG gives:

declaration is incompatible with function template "void foo<<unnamed>>()"

I *think* what we are issuing is closer in spirit to the latter, we 
don't get fooled into thinking it's an instantiation and we say that as 
a declaration doesn't work either. See what I mean? Removing completely 
the diagnostic code doesn't seem fine either because we end up with very 
confusing wordings like

variable or field declared void

or worse we mention variable templates, depending on the type (I already 
mentioned this).

Thus, all in all, would it make sense to simply issue something closer 
to EDG?

Thanks, Paolo.

Re: [C++ Patch] Improve grokdeclarator error

[Jason Merrill]

On Mon, Sep 30, 2019 at 6:54 AM Paolo Carlini <paolo.carlini@oracle.com> wrote:
>
> On 28/09/19 04:05, Jason Merrill wrote:
> > On 9/26/19 3:39 PM, Paolo Carlini wrote:
> >> +template void foo<int>(  // { dg-error "15:template-id .foo<int>.
> >> used as a declarator" }
> >
> > That's a strange diagnostic message; there's nothing wrong with using
> > a template-id as a declarator.  Why are we even calling grokdeclarator
> > when we hit EOF in the middle of the declarator?
>
> It's indeed a weird situation. Note, by the way, that for
>
> template void foo<int>;
>
> we end up giving the same diagnostic, only, the location was already fine.
>
> Anyway, to explain why I say weird, clang believes it's dealing with an
> explicit instantiation:
>
> explicit instantiation of 'foo' does not refer to a function template,
> variable template, member function, member class, or static data member
>
> whereas EDG gives:
>
> declaration is incompatible with function template "void foo<<unnamed>>()"
>
> I *think* what we are issuing is closer in spirit to the latter, we
> don't get fooled into thinking it's an instantiation and we say that as
> a declaration doesn't work either. See what I mean?

Well, it is an explicit instantiation, the problem (as EDG says) is
that the instantiation declaration doesn't match the template.

> Removing completely
> the diagnostic code doesn't seem fine either because we end up with very
> confusing wordings like
>
> variable or field declared void
>
> or worse we mention variable templates, depending on the type (I already
> mentioned this).

Any thought about my question about why we're calling grokdeclarator
in the first place?

> Thus, all in all, would it make sense to simply issue something closer
> to EDG?

Probably.

Jason

Re: [C++ Patch] Improve grokdeclarator error

[Paolo Carlini]

Hi,

On 30/09/19 20:46, Jason Merrill wrote:
> On Mon, Sep 30, 2019 at 6:54 AM Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> On 28/09/19 04:05, Jason Merrill wrote:
>>> On 9/26/19 3:39 PM, Paolo Carlini wrote:
>>>> +template void foo<int>(  // { dg-error "15:template-id .foo<int>.
>>>> used as a declarator" }
>>> That's a strange diagnostic message; there's nothing wrong with using
>>> a template-id as a declarator.  Why are we even calling grokdeclarator
>>> when we hit EOF in the middle of the declarator?
>> It's indeed a weird situation. Note, by the way, that for
>>
>> template void foo<int>;
>>
>> we end up giving the same diagnostic, only, the location was already fine.
>>
>> Anyway, to explain why I say weird, clang believes it's dealing with an
>> explicit instantiation:
>>
>> explicit instantiation of 'foo' does not refer to a function template,
>> variable template, member function, member class, or static data member
>>
>> whereas EDG gives:
>>
>> declaration is incompatible with function template "void foo<<unnamed>>()"
>>
>> I *think* what we are issuing is closer in spirit to the latter, we
>> don't get fooled into thinking it's an instantiation and we say that as
>> a declaration doesn't work either. See what I mean?
> Well, it is an explicit instantiation, the problem (as EDG says) is
> that the instantiation declaration doesn't match the template.

Definitely it doesn't.

>
>> Removing completely
>> the diagnostic code doesn't seem fine either because we end up with very
>> confusing wordings like
>>
>> variable or field declared void
>>
>> or worse we mention variable templates, depending on the type (I already
>> mentioned this).
> Any thought about my question about why we're calling grokdeclarator
> in the first place?

Yes. If you look at cp_parser_explicit_instantiation, clearly 
cp_parser_declarator doesn't return a cp_error_declarator and in such 
cases we always call grokdeclarator. Do you think we should error out 
here instead, much earlier? Currently grokdeclarator does quite a bit of 
work before the diagnostic.

Paolo..

Re: [C++ Patch] Improve grokdeclarator error

[Jason Merrill]

On Mon, Sep 30, 2019 at 3:25 PM Paolo Carlini <paolo.carlini@oracle.com> wrote:
>
> On 30/09/19 20:46, Jason Merrill wrote:
> > On Mon, Sep 30, 2019 at 6:54 AM Paolo Carlini <paolo.carlini@oracle.com> wrote:
> >> On 28/09/19 04:05, Jason Merrill wrote:
> >>> On 9/26/19 3:39 PM, Paolo Carlini wrote:
> >>>> +template void foo<int>(  // { dg-error "15:template-id .foo<int>.
> >>>> used as a declarator" }
> >>> That's a strange diagnostic message; there's nothing wrong with using
> >>> a template-id as a declarator.  Why are we even calling grokdeclarator
> >>> when we hit EOF in the middle of the declarator?
> >> It's indeed a weird situation. Note, by the way, that for
> >>
> >> template void foo<int>;
> >>
> >> we end up giving the same diagnostic, only, the location was already fine.
> >>
> >> Anyway, to explain why I say weird, clang believes it's dealing with an
> >> explicit instantiation:
> >>
> >> explicit instantiation of 'foo' does not refer to a function template,
> >> variable template, member function, member class, or static data member
> >>
> >> whereas EDG gives:
> >>
> >> declaration is incompatible with function template "void foo<<unnamed>>()"
> >>
> >> I *think* what we are issuing is closer in spirit to the latter, we
> >> don't get fooled into thinking it's an instantiation and we say that as
> >> a declaration doesn't work either. See what I mean?
> > Well, it is an explicit instantiation, the problem (as EDG says) is
> > that the instantiation declaration doesn't match the template.
>
> Definitely it doesn't.
>
> >
> >> Removing completely
> >> the diagnostic code doesn't seem fine either because we end up with very
> >> confusing wordings like
> >>
> >> variable or field declared void
> >>
> >> or worse we mention variable templates, depending on the type (I already
> >> mentioned this).
> > Any thought about my question about why we're calling grokdeclarator
> > in the first place?
>
> Yes. If you look at cp_parser_explicit_instantiation, clearly
> cp_parser_declarator doesn't return a cp_error_declarator and in such
> cases we always call grokdeclarator. Do you think we should error out
> here instead, much earlier? Currently grokdeclarator does quite a bit of
> work before the diagnostic.

I would expect a cp_error_declarator for this case.

Jason