On Mon, Feb 4, 2019 at 4:52 PM Marek Polacek <pola...@redhat.com> wrote:
> On Mon, Feb 04, 2019 at 09:44:01AM -0500, Jason Merrill wrote:
> > On 2/1/19 2:55 PM, Marek Polacek wrote:
> > > On Fri, Feb 01, 2019 at 12:02:44PM -0500, Jason Merrill wrote:
> > > > On 2/1/19 11:26 AM, Marek Polacek wrote:
> > > > > On Wed, Jan 30, 2019 at 01:39:11PM -0500, Jason Merrill wrote:
> > > > > > On 1/28/19 9:46 PM, Marek Polacek wrote:
> > > > > > > This patch fixes an ICE-on-invalid (becase out-of-line
> > > > > > > constructors can't have
> > > > > > > template arguments and also because function templates can't be
> > > > > > > partially
> > > > > > > specialized) in C++2a: when we're parsing
> > > > > > >
> > > > > > > template<typename T> template<typename U> A<T>::A<U> ()
> > > > > > >
> > > > > > > in the attached test we end up parsing "A<T>::A<U>" as a type
> > > > > > > name, and first we
> > > > > > > try a class-name. First we process "A<T>::" as the nested name
> > > > > > > specifier and then
> > > > > > > we parse "A<U>". In this test that results in a BASELINK.
> > > > > > > Because in this context
> > > > > > > we're supposed to treat it as a typename ([temp.res]/6), we call
> > > > > > > make_typename_type,
> > > > > > > but that crashes.
> > > > > >
> > > > > > Hmm. If we've done an actual lookup (that gave us a BASELINK), we
> > > > > > aren't
> > > > > > dealing with a member of an unknown specialization anymore, so we
> > > > > > should
> > > > > > just use the result of the lookup rather than speculate about what
> > > > > > the name
> > > > > > might mean. Why are we still trying to treat it as a typename?
> > > > >
> > > > > Good point. It's because cp_parser_class_name has:
> > > > >
> > > > > 23095 /* Any name names a type if we're following the `typename'
> > > > > keyword
> > > > > 23096 in a qualified name where the enclosing scope is
> > > > > type-dependent. */
> > > > > 23097 typename_p = (typename_keyword_p && scope && TYPE_P (scope)
> > > > > 23098 && dependent_type_p (scope));
> > > > >
> > > > > and scope is in this case "A<T>" which is dependent. Then there's
> > > > > this
> > > > > "identifier, but not template-id" case which only performs name
> > > > > lookup when
> > > > > typename_p is false. But we're parsing "A<U>" so we call
> > > > > cp_parser_template_id. It sees CPP_TEMPLATE_ID -- an already parsed
> > > > > template-id, so it just returns it, which is a BASELINK. So even
> > > > > messing
> > > > > with tag_type wouldn't help.
> > > > >
> > > > > Does this answer your question?
> > > >
> > > > Mostly, though I'm still curious how we got the previous parse of the
> > > > template-id and yet continued to try to parse it as a class-name.
> > >
> > > So we have
> > > template<typename T> template<typename U>
> > > A<T>::A<U> ()
> > >
> > > and we're in cp_parser_single_declaration. We're trying to parse the
> > > decl-specifier-seq, and that first tries to parse variou RID_* keywords
> > > and if that doesn't work it tries to parse a constructor:
> > >
> > > /* Constructors are a special case. The `S' in `S()' is not a
> > > decl-specifier; it is the beginning of the declarator. */
> > > constructor_p
> > > = (!found_decl_spec
> > > && constructor_possible_p
> > > && (cp_parser_constructor_declarator_p
> > > (parser, decl_spec_seq_has_spec_p (decl_specs, ds_friend))));
> > >
> > > cp_parser_constructor_declarator_p calls
> > > cp_parser_nested_name_specifier_opt
> > > and that's where we parse the two template-ids, A<T> and A<U>. But
> > > cp_parser_constructor_declarator_p isn't successful so we go to parsing
> > > A<T>::A<U> as a type-specifier, and that's where the above happens.
> >
> > Aha. That seems like a bug in cp_parser_constructor_declarator.
>
> Sorry, what specifically, that cp_parser_constructor_declarator_p thinks it's
> not a ctor? I thought it was an invalid ctor, since "out-of-line constructor
> for 'A' cannot have template arguments".
True, good point, though it is still more a constructor declarator
than anything else. It would be nice for the diagnostic to suggest
removing the redundant template arguments aren't allowed, not just the
current confusing message about a partial specialization. The
diagnostic for a similar declaration of a nested class template is
"partial specialization ‘struct A<T>::B<U>’ does not specialize any
template arguments; to define the primary template, remove the
template argument list".
We also shouldn't mention the internal name "__ct".
> Once again, we're parsing "A<T>::A<U> ()". "A<T>::" is parsed as a nested-
> name-specifier = struct A. Then:
>
> 27377 /* If we have a class scope, this is easy; DR 147 says that S::S
> always
> 27378 names the constructor, and no other qualified name could. */
> 27379 if (constructor_p && nested_name_specifier
> 27380 && CLASS_TYPE_P (nested_name_specifier))
> 27381 {
> 27382 tree id = cp_parser_unqualified_id (parser,
> 27383 /*template_keyword_p=*/false,
> 27384 /*check_dependency_p=*/false,
> 27385 /*declarator_p=*/true,
> 27386 /*optional_p=*/false);
> 27387 if (is_overloaded_fn (id))
> 27388 id = DECL_NAME (get_first_fn (id));
> 27389 if (!constructor_name_p (id, nested_name_specifier))
> 27390 constructor_p = false;
> 27391 }
>
> 'id' is a BASELINK, we extract its name which is "__ct". And
> constructor_name_p
> returns false because __ct != A. What would you like we do here?
__ct is ctor_identifier, which certainly names a constructor, and we
got this BASELINK by looking up the name of the class, so I think
returning true would be more appropriate.
Jason
