On 06/01/2014 18:29, Richard Smith wrote:
On Mon Jan 06 2014 at 10:06:25 AM, Alp Toker <[email protected]
<mailto:[email protected]>> 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">;
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?
Alp.
The thought experiment here is that
ExpectAndConsume(tok::annot_template_id) would emit the correct
diagnostic message, which it hopefully does at this point (other than
that it would assert right now due to the special token check).
> , particularly because the comment says it's intended for use
with function template names.
That's an interesting point but I didn't quite understand. Can you
explain a bit further?
>
> I'd rather have two variants for this diagnostic, or use an enum
specifically marked as "keep in sync with this diagnostic"
(slightly better than 0-1-2-3-4). Is there a reason you
specifically wanted to change this one?
Chose this diagnostic to demonstrate the new diagnostic printing
facility introduced in the previous commit. It looked like a good
candidate to upgrade first because of the odd use of tok::kw_template.
The idea with the ongoing diagnostic effort is to make the
messages more
consistent, clearly written and easy to maintain so if it's
missing the
mark we should deal with that.
Alp.
>
> Jordan
>
>
>
> On Jan 6, 2014, at 4:54 , Alp Toker <[email protected]
<mailto:[email protected]>> wrote:
>
>> Author: alp
>> Date: Mon Jan 6 06:54:32 2014
>> New Revision: 198605
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=198605&view=rev
>> Log:
>> Don't use magic constants in the digraph diagnostic
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> cfe/trunk/lib/Parse/ParseExprCXX.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=198605&r1=198604&r2=198605&view=diff
>>
==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
(original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon
Jan 6 06:54:32 2014
>> @@ -641,9 +641,7 @@ def err_ctor_init_missing_comma : Error<
>> def err_friend_decl_defines_type : Error<
>> "cannot define a type in a friend declaration">;
>> def err_missing_whitespace_digraph : Error<
>> - "found '<::' after a "
>> - "%select{template
name|const_cast|dynamic_cast|reinterpret_cast|static_cast}0"
>> - " which forms the digraph '<:' (aka '[') and a ':', did you
mean '< ::'?">;
>> + "found '<::' after a %0 which forms the digraph '<:' (aka
'[') and a ':', did you mean '< ::'?">;
>>
>> def ext_deleted_function : ExtWarn<
>> "deleted function definitions are a C++11 extension">,
InGroup<CXX11>;
>>
>> Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
>> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=198605&r1=198604&r2=198605&view=diff
>>
==============================================================================
>> --- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Mon Jan 6 06:54:32 2014
>> @@ -24,18 +24,6 @@
>>
>> using namespace clang;
>>
>> -static int SelectDigraphErrorMessage(tok::TokenKind Kind) {
>> - switch (Kind) {
>> - case tok::kw_template: return 0;
>> - case tok::kw_const_cast: return 1;
>> - case tok::kw_dynamic_cast: return 2;
>> - case tok::kw_reinterpret_cast: return 3;
>> - case tok::kw_static_cast: return 4;
>> - default:
>> - llvm_unreachable("Unknown type for digraph error message.");
>> - }
>> -}
>> -
>> // Are the two tokens adjacent in the same source file?
>> bool Parser::areTokensAdjacent(const Token &First, const Token
&Second) {
>> SourceManager &SM = PP.getSourceManager();
>> @@ -56,8 +44,7 @@ static void FixDigraph(Parser &P, Prepro
>> Range.setBegin(DigraphToken.getLocation());
>> Range.setEnd(ColonToken.getLocation());
>> P.Diag(DigraphToken.getLocation(),
diag::err_missing_whitespace_digraph)
>> - << SelectDigraphErrorMessage(Kind)
>> - << FixItHint::CreateReplacement(Range, "< ::");
>> + << Kind << FixItHint::CreateReplacement(Range, "< ::");
>>
>> // Update token information to reflect their change in token
type.
>> ColonToken.setKind(tok::coloncolon);
>> @@ -93,8 +80,8 @@ void Parser::CheckForTemplateAndDigraph(
>> Template,
MemberOfUnknownSpecialization))
>> return;
>>
>> - FixDigraph(*this, PP, Next, SecondToken, tok::kw_template,
>> - /*AtDigraph*/false);
>> + FixDigraph(*this, PP, Next, SecondToken, tok::annot_template_id,
>> + /*AtDigraph*/ false);
>> }
>>
>> /// \brief Emits an error for a left parentheses after a double
colon.
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> [email protected] <mailto:[email protected]>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected] <mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
