r198605 - Don't use magic constants in the digraph diagnostic

[Jordan Rose]

On Jan 6, 2014, at 18:14, Alp Toker <alp at nuanti.com> wrote:

> 
> On 06/01/2014 19:11, Richard Smith wrote:
>> On Mon Jan 06 2014 at 11:01:02 AM, Alp Toker <alp at nuanti.com <mailto:alp at nuanti.com>> wrote:
>> 
>> 
>>    On 06/01/2014 18:29, Richard Smith wrote:
>>    > On Mon Jan 06 2014 at 10:06:25 AM, Alp Toker <alp at nuanti.com
>>    <mailto:alp at nuanti.com>
>>    > <mailto:alp at nuanti.com <mailto:alp at nuanti.com>>> wrote:
>>    >
>>    >
>>    >     On 06/01/2014 17:34, Jordan Rose wrote:
>>    >     > This changes the message for templates in a way that is less
>>    >     informative.
>>    >
>>    >     No change in output. Do you mean the string in the TableGen file
>>    >     is now
>>    >     less informative?
>>    >
>>    >     > In other cases where we use tokens as diagnostic
>>    arguments, the
>>    >     message makes sense whether or not the token text is quoted.
>>    Here,
>>    >     that's not true: the problem isn't about the literal word
>>    >     "template" but about a particular template (like "vector").
>>    Using
>>    >     tok::annot_template_id as if it were a real token seems wrong
>>    >
>>    >     That was the reason I changed the "magic token kind" in use from
>>    >     tok::kw_template to tok::annot_template_id. tok::kw_template
>>    >     relates to
>>    >     "template" whereas tok::annot_template_id is a synonym for
>>    "template
>>    >     name" so this appears a better choice to use as the placeholder.
>>    >
>>    >
>>    > It's a synonym for 'template id' (a template name followed by
>>    template
>>    > arguments), not for 'template name'. Using it here is inappropriate.
>> 
>>    Diagnostics seem to call it that way, with 'template-id' not appearing
>>    rarely:
>> 
>>    def err_template_id_not_a_type : Error<
>>       "template name refers to non-type template %0">;
>> 
>> 
>> This looks clear and technically correct to me. We're diagnosing that the template-id X<Y> is not a type by saying that X is a non-type template name.
>> 
>>    I agree it's not fully appropriate, but it's more appropriate than the
>>    previous tok::kw_template given that there isn't a "template"
>>    keyword in
>>    sight:
>> 
>>    D<::F> A2; // expected-error{{found '<::' after a template name which
>>    forms the digraph}}
>> 
>>    Can you think of a better alternative to either?
>> 
>> 
>> Jordan's suggestion of using an enum seems fine to me. (As Aaron Ballman and I have previously discussed on-list, we should also have a mechanism to generate these enums from the .td files to avoid the magic numbers entirely.)
> 
> Even with an enum there's still going to have to be a switch somewhere, and what I've really been looking for is boilerplate-free way to share code construct descriptions with the parser, sema and code completion.
> 
> Agree that this change might not be the solution (especially annot_type_id) but the previous code was at least as dubious with its kw_template so I'm not keen on bringing that back to life either.
> 
> So if it's OK with Jordan I'd like to mull over this for a few days to see if we can move it forward in a satisfactory way first. Will examine those threads again too.
> 
> Alp.

The previous code was silly, but it was just using kw_template as a switch key. If you want to use tok::identifier there instead, that's fine, but I'd be happier if the structure were put back the way it originally was for now.

Jordan