From: Paolo Carlini <paolo.carlini@oracle.com>
To: Jason Merrill <jason@redhat.com>
Cc: gcc-patches List <gcc-patches@gcc.gnu.org>,
Nathan Sidwell <nathan@acm.org>
Subject: Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
Date: Fri, 14 Dec 2018 21:34:00 -0000 [thread overview]
Message-ID: <a1641f51-e78d-898f-3a7e-1869a4889b5e@oracle.com> (raw)
In-Reply-To: <aa702238-8bcf-e268-0408-fc2050b1287e@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4488 bytes --]
Hi,
On 14/12/18 21:19, Jason Merrill wrote:
> On 12/14/18 1:44 PM, Paolo Carlini wrote:
>> Hi,
>>
>> On 13/12/18 22:03, Jason Merrill wrote:
>>> On 10/30/18 9:22 PM, Paolo Carlini wrote:
>>>> Hi,
>>>>
>>>> On 30/10/18 21:37, Jason Merrill wrote:
>>>>> On 10/26/18 2:02 PM, Paolo Carlini wrote:
>>>>>> On 26/10/18 17:18, Jason Merrill wrote:
>>>>>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini
>>>>>>> <paolo.carlini@oracle.com> wrote:
>>>>>>>> On 24/10/18 22:41, Jason Merrill wrote:
>>>>>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote:
>>>>>>>>>> Â Â Â Â Â Â Â Â && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE
>>>>>>>>>> +Â Â Â Â Â Â && TREE_CODE (declspecs->type) != DECLTYPE_TYPE
>>>>>>>>>> Â Â Â Â Â Â Â Â Â && MAYBE_CLASS_TYPE_P (declspecs->type))
>>>>>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be
>>>>>>>>> CLASS_TYPE_P,
>>>>>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to
>>>>>>>>> allow template type parameters for some reason?
>>>>>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However
>>>>>>>> it seems
>>>>>>>> we at least want to let through TEMPLATE_TYPE_PARMs
>>>>>>>> representing 'auto'
>>>>>>>> - otherwise Dodji's check a few lines below which fixed c++/51473
>>>>>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM,
>>>>>>>> otherwise
>>>>>>>> we regress on template/spec32.C and template/ttp22.C because we
>>>>>>>> don't
>>>>>>>> diagnose the shadowing anymore. Thus, I would say either we
>>>>>>>> keep on
>>>>>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we
>>>>>>>> add a comment?
>>>>>>> Aha. I guess the answer is not to restrict that test any more, but
>>>>>>> instead to fix the code further down so it gives a proper
>>>>>>> diagnostic
>>>>>>> rather than call warn_misplaced_attr_for_class_type.
>>>>>>
>>>>>> I see. Thus something like the below? It passes testing on
>>>>>> x86_64-linux.
>>>>>
>>>>>> +Â if ((!declared_type || TREE_CODE (declared_type) ==
>>>>>> DECLTYPE_TYPE)
>>>>>> +Â Â Â Â Â && ! saw_friend && !error_p)
>>>>>> Â Â Â Â permerror (input_location, "declaration does not declare
>>>>>> anything");
>>>>>
>>>>> I see no reason to make this specific to decltype. Maybe move
>>>>> this diagnostic into the final 'else' block with the other
>>>>> declspec diagnostics and not look at declared_type at all?
>>>>
>>>> I'm not sure to fully understand: if we do that we still want to at
>>>> least minimally check that declared_type is null, like we already
>>>> do, and then we simply accept the new testcase. Is that Ok?
>>>> Because, as I probably mentioned at some point, all the other
>>>> compilers I have at hand issue a "does not declare anything"
>>>> diagnostic, and we likewise do that for the legacy __typeof. Not
>>>> looking into declared_type *at all* doesn't work with plain class
>>>> types and enums, of course. Or you meant something entirely
>>>> different??
>>>>
>>>>>> +Â if (declspecs->attributes && warn_attributes && declared_type
>>>>>> +Â Â Â Â Â && TREE_CODE (declared_type) != DECLTYPE_TYPE)
>>>>>
>>>>> I think we do want to give a diagnostic about useless attributes,
>>>>> not skip it.
>>>>
>>>> Agreed. FWIW the attached tests fine.
>>>
>>> The problem here is that the code toward the bottom expects
>>> "declared_type" to be the tagged type declared by a declaration with
>>> no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE.
>>>
>>> I think once we've checked for 'auto' we don't want declared_type to
>>> be anything that isn't OVERLOAD_TYPE_P. We can arrange that either
>>> by checking for 'auto' first and then changing the code that sets
>>> declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type
>>> after checking for 'auto' if it isn't OVERLOAD_TYPE_P.
>>
>> Thanks. I'm slowly catching up on this issue... Any suggestion about
>> BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes
>> - which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we
>> regress on template/spec32.C, we don't reject it anymore.
> If we clear declared_type for a BOUND_TEMPLATE_TEMPLATE_PARM, we
> should get the "does not declare anything" error.
Ah, now I see, I didn't realize that we would also change the errors we
issue for those testcases. Thus the below is finishing testing, appears
to work fine.
Thanks, Paolo.
///////////////////
[-- Attachment #2: patch_84644_3 --]
[-- Type: text/plain, Size: 3075 bytes --]
Index: cp/decl.c
===================================================================
--- cp/decl.c (revision 267131)
+++ cp/decl.c (working copy)
@@ -4803,9 +4803,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
declared_type = declspecs->type;
else if (declspecs->type == error_mark_node)
error_p = true;
- if (declared_type == NULL_TREE && ! saw_friend && !error_p)
- permerror (input_location, "declaration does not declare anything");
- else if (declared_type != NULL_TREE && type_uses_auto (declared_type))
+
+ if (type_uses_auto (declared_type))
{
error_at (declspecs->locations[ds_type_spec],
"%<auto%> can only be specified for variables "
@@ -4812,6 +4811,12 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
"or function declarations");
return error_mark_node;
}
+
+ if (declared_type && !OVERLOAD_TYPE_P (declared_type))
+ declared_type = NULL_TREE;
+
+ if (!declared_type && !saw_friend && !error_p)
+ permerror (input_location, "declaration does not declare anything");
/* Check for an anonymous union. */
else if (declared_type && RECORD_OR_UNION_CODE_P (TREE_CODE (declared_type))
&& TYPE_UNNAMED_P (declared_type))
Index: testsuite/g++.dg/cpp0x/decltype-33838.C
===================================================================
--- testsuite/g++.dg/cpp0x/decltype-33838.C (revision 267127)
+++ testsuite/g++.dg/cpp0x/decltype-33838.C (working copy)
@@ -2,5 +2,5 @@
// PR c++/33838
template<typename T> struct A
{
- __decltype (T* foo()); // { dg-error "expected|no arguments|accept" }
+ __decltype (T* foo()); // { dg-error "expected|no arguments|declaration" }
};
Index: testsuite/g++.dg/cpp0x/decltype68.C
===================================================================
--- testsuite/g++.dg/cpp0x/decltype68.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/decltype68.C (working copy)
@@ -0,0 +1,7 @@
+// PR c++/84644
+// { dg-do compile { target c++11 } }
+
+template<int a>
+struct b {
+ decltype(a) __attribute__((break)); // { dg-error "declaration does not declare anything" }
+};
Index: testsuite/g++.dg/template/spec32.C
===================================================================
--- testsuite/g++.dg/template/spec32.C (revision 267127)
+++ testsuite/g++.dg/template/spec32.C (working copy)
@@ -2,5 +2,5 @@
struct A
{
- template<template<int> class B> struct B<0>; // { dg-error "name of class shadows" }
+ template<template<int> class B> struct B<0>; // { dg-error "declaration does not declare anything" }
};
Index: testsuite/g++.dg/template/ttp22.C
===================================================================
--- testsuite/g++.dg/template/ttp22.C (revision 267127)
+++ testsuite/g++.dg/template/ttp22.C (working copy)
@@ -2,7 +2,7 @@
// { dg-do compile }
template<template<int> class A>
-class A<0>; // { dg-error "shadows template template parameter" }
+class A<0>; // { dg-error "declaration does not declare anything" }
template<template<int> class B>
class B<0> {}; // { dg-error "shadows template template parameter" }
next prev parent reply other threads:[~2018-12-14 21:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-15 18:21 Paolo Carlini
2018-10-24 21:56 ` Jason Merrill
2018-10-26 9:37 ` Paolo Carlini
2018-10-26 16:28 ` Jason Merrill
2018-10-26 19:03 ` Paolo Carlini
2018-10-30 21:46 ` Jason Merrill
2018-10-31 5:25 ` Paolo Carlini
2018-12-13 22:22 ` Jason Merrill
2018-12-14 18:44 ` Paolo Carlini
2018-12-14 20:19 ` Jason Merrill
2018-12-14 21:34 ` Paolo Carlini [this message]
2018-12-14 21:43 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a1641f51-e78d-898f-3a7e-1869a4889b5e@oracle.com \
--to=paolo.carlini@oracle.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=nathan@acm.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).