On 3/23/20 12:50 PM, Martin Sebor wrote:
On 3/23/20 8:49 AM, Jason Merrill wrote:
On 3/21/20 5:59 PM, Martin Sebor wrote:
+ /* Diagnose class/struct/union mismatches. IS_DECLARATION is
false
+ for alias definition. */
+ bool decl_class = (is_declaration
+ && cp_parser_declares_only_class_p (parser));
cp_parser_check_class_key (parser, key_loc, tag_type, type,
false,
cp_parser_declares_only_class_p (parser));
Don't you need to use the new variable?
Don't your testcases exercise this?
Of course they do. This was a leftover from an experiment after I put
the initial updated patch together. On final review I decided to adjust
some comments and forgot to restore the original use of the variable.
+ /* When TYPE is the use of an implicit specialization of a
previously
+ declared template set TYPE_DECL to the type of the primary
template
+ for the specialization and look it up in CLASS2LOC below. For
uses
+ of explicit or partial specializations TYPE_DECL already points to
+ the declaration of the specialization.
+ IS_USE is clear so that the type of an implicit instantiation
rather
+ than that of a partial specialization is determined. */
+ type_decl = TREE_TYPE (type_decl);
+ if (TREE_CODE (type_decl) != TEMPLATE_DECL)
+ type_decl = TYPE_MAIN_DECL (type_decl);
The comment is no longer relevant to the code. The remaining code
also seems like it would have no effect; we already know type_decl is
TYPE_MAIN_DECL (type).
I removed the block of code.
Martin
PS I would have preferred to resolve just the reported problem in this
patch and deal with the template specializations more fully (and with
aliases) in a followup. As it is, it has grown bigger and more complex
than I'm comfortable with, especially with the template specializations,
harder for me to follow, and obviously a lot more time-consuming not
just to put together but also to review. Although this revision handles
many more template specialization cases correctly, there still are other
(arguably corner) cases that it doesn't. I suspect getting those right
might even require a design change, which I see as out of scope at this
time (not to mention my ability).
Sure, at this point in the cycle there's always a tradeoff between
better functionality and risk from ballooning changes. It looks like
the improved template handling could still be split out into a separate
patch, if you'd prefer.
+ /* Number of usesn of the class. */
Typo.
+ definintion if one exists or the first declaration otherwise. */
typo.
+ if (CLASSTYPE_USE_TEMPLATE (type) == 1 && !is_decl (0))
...
+ the first reference to the instantiation. The primary must
+ be (and inevitably is) at index zero. */
I think CLASSTYPE_IMPLICIT_INSTANTIATION is more readable than testing
the value 1.
What is the !is_decl (0) intended to do? Changing it to an assert
inside the 'if' doesn't seem to affect any of the testcases.
+ For implicit instantiations of a primary template it's
+ the class-key used to declare the primary with. The primary
+ must be at index zero. */
+ const tag_types xpect_key
+ = cdlguide->class_key (cdlguide == this ? idxguide : 0);
A template can also be declared before it's defined; I think you want to
move the def_p/idxdef/idxguide logic into another member function that
you invoke on cdlguide to perhaps get the class_key_loc_t that you want
to use as the pattern.
Jason