[PATCH 3/3] change class-key of PODs to struct and others to class (PR 61339)

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On Tue, Jul 9, 2019 at 8:34 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 7/9/19 9:17 AM, Richard Sandiford wrote:
>> > Martin Sebor <msebor@gmail.com> writes:
>> >> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>> >> index cfc41e1ed86..625d5b17413 100644
>> >> --- a/gcc/cp/cp-tree.h
>> >> +++ b/gcc/cp/cp-tree.h
>> >> @@ -6428,7 +6428,7 @@ extern tree get_scope_of_declarator            (const cp_declarator *);
>> >>   extern void grok_special_member_properties (tree);
>> >>   extern bool grok_ctor_properties           (const_tree, const_tree);
>> >>   extern bool grok_op_properties                     (tree, bool);
>> >> -extern tree xref_tag                                (enum tag_types, tree, tag_scope, bool, bool * = NULL);
>> >> +extern tree xref_tag                                (enum tag_types, tree, tag_scope, bool);
>> >>   extern tree xref_tag_from_type                     (tree, tree, tag_scope);
>> >>   extern void xref_basetypes                 (tree, tree);
>> >>   extern tree start_enum                             (tree, tree, tree, tree, bool, bool *);
>> >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
>> >> index 005f99a6e15..9accc3d141b 100644
>> >> --- a/gcc/cp/decl.c
>> >> +++ b/gcc/cp/decl.c
>> >> @@ -14119,7 +14119,7 @@ lookup_and_check_tag (enum tag_types tag_code, tree name,
>> >>
>> >>   static tree
>> >>   xref_tag_1 (enum tag_types tag_code, tree name,
>> >> -            tag_scope scope, bool template_header_p, bool *new_p)
>> >> +            tag_scope scope, bool template_header_p)
>> >>   {
>> >>     enum tree_code code;
>> >>     tree context = NULL_TREE;
>> >> @@ -14151,9 +14151,6 @@ xref_tag_1 (enum tag_types tag_code, tree name,
>> >>     if (t == error_mark_node)
>> >>       return error_mark_node;
>> >>
>> >> -  /* Let the caller know this is a new type.  */
>> >> -  *new_p = t == NULL_TREE;
>> >> -
>> >>     if (scope != ts_current && t && current_class_type
>> >>         && template_class_depth (current_class_type)
>> >>         && template_header_p)
>> >> @@ -14215,7 +14212,6 @@ xref_tag_1 (enum tag_types tag_code, tree name,
>> >>            scope = ts_current;
>> >>          }
>> >>        t = pushtag (name, t, scope);
>> >> -      *new_p = true;
>> >>      }
>> >>       }
>> >>     else
>> >> @@ -14267,13 +14263,11 @@ xref_tag_1 (enum tag_types tag_code, tree name,
>> >>
>> >>   tree
>> >>   xref_tag (enum tag_types tag_code, tree name,
>> >> -          tag_scope scope, bool template_header_p, bool *new_p /* = NULL */)
>> >> +          tag_scope scope, bool template_header_p)
>> >>   {
>> >>     bool dummy;
>> >> -  if (!new_p)
>> >> -    new_p = &dummy;
>> >>     bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
>> >> -  tree ret = xref_tag_1 (tag_code, name, scope, template_header_p, new_p);
>> >> +  tree ret = xref_tag_1 (tag_code, name, scope, template_header_p);
>> >>     timevar_cond_stop (TV_NAME_LOOKUP, subtime);
>> >>     return ret;
>> >>   }
>> >> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>> >> index 52af8c0c6d6..d16bf253058 100644
>> >> --- a/gcc/cp/parser.c
>> >> +++ b/gcc/cp/parser.c
>> >> @@ -28193,8 +28193,6 @@ cp_parser_template_declaration_after_parameters (cp_parser* parser,
>> >>                                         member_p,
>> >>                                              /*explicit_specialization_p=*/false,
>> >>                                         &friend_p);
>> >> -      // maybe_warn_struct_vs_class (token->location, TREE_TYPE (decl));
>> >> -
>> >>         pop_deferring_access_checks ();
>> >>
>> >>         /* If this is a member template declaration, let the front
>> >
>> > Looks like this might have been part of 1/3.
>>
>> Yes, this and a few other hunks didn't belong in this patch.
>> I removed them, retested the patch, and committed r273311.
>>
>> >
>> > OK otherwise.  Thanks again for doing this.
>> >
>> > (I guess a lot of these tags could be removed, but that was just as true
>> > before the patch, so it's still a strict improvement.)
>>
>> Most could be removed and my own preference would have been to
>> remove them.  The warning has a mechanism for figuring out which
>> ones can one can go and which ones are needed and I considered
>> making use of it.  In the end I decided to be conservative and
>> keep them in case someone preferred it that way.  Making
>> the change now that the cleanup is done will be slightly more
>> involved.  I suppose we could add yet another warning to find
>> them: -Wredundant-tag.
>
> Just to pick one - why is struct loop not a POD?  Because of its
> widest_int members?

Yeah.

> But then we allocate it with ggc_cleared_alloc<class loop> () which
> AFAICS doesn't invoke a constructor

Yeah, here and elsewhere we have a habit of initialising non-PODs
without using the proper constructor.

> (and I hope it doesn't trigger the finalization path).

Yeah, but that's based on whether it has a trivial destructor rather
than whether it's POD.

Thanks,
Richard