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

[Jordan Rose]

This changes the message for templates in a way that is 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, particularly because the comment says it's intended for use with function template names.

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?

Jordan



On Jan 6, 2014, at 4:54 , Alp Toker <alp at nuanti.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits