Change TokenKind's Print overload to a format_provider. (#2534)

Fundamentally this `.Print()` is wrong for debug output at present because `.fixed_spelling()` can be empty. It's also inconsistent with other enums to use it. We frequently print tokens for debugging, and it's easy to forget to specify `.name()` there.

Diagnostics use formatv, so we can provide a format_provider and address it in one spot that way. It also makes it harder to just forget to do the right thing.

toolchain/diagnostics/diagnostic_emitter.h

@@ -168,6 +168,9 @@ struct DiagnosticBase {
 
  private:
   // Handles the cast of llvm::Any to Args types for formatv.
+  // TODO: Custom formatting can be provided with an format_provider, but that
+  // affects all formatv calls. Consider replacing formatv with a custom call
+  // that allows diagnostic-specific formatting.
   template <std::size_t... N>
   inline auto FormatFnImpl(const DiagnosticMessage& message,
                            std::index_sequence<N...> /*indices*/) const

toolchain/lexer/BUILD

@@ -13,7 +13,6 @@ cc_library(
     textual_hdrs = ["token_kind.def"],
     deps = [
         "//common:check",
-        "//common:ostream",
         "//toolchain/common:enum_base",
         "@llvm-project//llvm:Support",
     ],

toolchain/lexer/token_kind.h

@@ -7,7 +7,7 @@
 
 #include <cstdint>
 
-#include "common/ostream.h"
+#include "llvm/Support/FormatVariadicDetails.h"
 #include "toolchain/common/enum_base.h"
 
 namespace Carbon {
@@ -70,11 +70,6 @@ class TokenKind : public CARBON_ENUM_BASE(TokenKind) {
     }
     return false;
   }
-
-  // Override the EnumBase printing to use the fixed spelling rather than the
-  // name for tokens as this better corresponds to the source code the
-  // represent.
-  void Print(llvm::raw_ostream& out) const { out << fixed_spelling(); }
 };
 
 #define CARBON_TOKEN(TokenName) \
@@ -83,4 +78,18 @@ class TokenKind : public CARBON_ENUM_BASE(TokenKind) {
 
 }  // namespace Carbon
 
+namespace llvm {
+
+// We use formatv primarily for diagnostics. In these cases, it's expected that
+// the spelling in source code should be used.
+template <>
+struct format_provider<Carbon::TokenKind> {
+  static void format(const Carbon::TokenKind& kind, raw_ostream& out,
+                     StringRef /*style*/) {
+    out << kind.fixed_spelling();
+  }
+};
+
+}  // namespace llvm
+
 #endif  // CARBON_TOOLCHAIN_LEXER_TOKEN_KIND_H_

toolchain/lexer/token_kind_test.cpp

@@ -27,14 +27,12 @@ constexpr llvm::StringLiteral KeywordRegex = "[a-z_]+|Self";
 
 #define CARBON_TOKEN(TokenName)                           \
   TEST(TokenKindTest, TokenName) {                        \
-    EXPECT_EQ(#TokenName, TokenKind::TokenName.name());   \
     EXPECT_FALSE(TokenKind::TokenName.is_symbol());       \
     EXPECT_FALSE(TokenKind::TokenName.is_keyword());      \
     EXPECT_EQ("", TokenKind::TokenName.fixed_spelling()); \
   }
 #define CARBON_SYMBOL_TOKEN(TokenName, Spelling)                \
   TEST(TokenKindTest, TokenName) {                              \
-    EXPECT_EQ(#TokenName, TokenKind::TokenName.name());         \
     EXPECT_TRUE(TokenKind::TokenName.is_symbol());              \
     EXPECT_FALSE(TokenKind::TokenName.is_grouping_symbol());    \
     EXPECT_FALSE(TokenKind::TokenName.is_opening_symbol());     \
@@ -45,7 +43,6 @@ constexpr llvm::StringLiteral KeywordRegex = "[a-z_]+|Self";
   }
 #define CARBON_OPENING_GROUP_SYMBOL_TOKEN(TokenName, Spelling, ClosingName)   \
   TEST(TokenKindTest, TokenName) {                                            \
-    EXPECT_EQ(#TokenName, TokenKind::TokenName.name());                       \
     EXPECT_TRUE(TokenKind::TokenName.is_symbol());                            \
     EXPECT_TRUE(TokenKind::TokenName.is_grouping_symbol());                   \
     EXPECT_TRUE(TokenKind::TokenName.is_opening_symbol());                    \
@@ -57,7 +54,6 @@ constexpr llvm::StringLiteral KeywordRegex = "[a-z_]+|Self";
   }
 #define CARBON_CLOSING_GROUP_SYMBOL_TOKEN(TokenName, Spelling, OpeningName)   \
   TEST(TokenKindTest, TokenName) {                                            \
-    EXPECT_EQ(#TokenName, TokenKind::TokenName.name());                       \
     EXPECT_TRUE(TokenKind::TokenName.is_symbol());                            \
     EXPECT_TRUE(TokenKind::TokenName.is_grouping_symbol());                   \
     EXPECT_FALSE(TokenKind::TokenName.is_opening_symbol());                   \
@@ -69,7 +65,6 @@ constexpr llvm::StringLiteral KeywordRegex = "[a-z_]+|Self";
   }
 #define CARBON_KEYWORD_TOKEN(TokenName, Spelling)               \
   TEST(TokenKindTest, TokenName) {                              \
-    EXPECT_EQ(#TokenName, TokenKind::TokenName.name());         \
     EXPECT_FALSE(TokenKind::TokenName.is_symbol());             \
     EXPECT_TRUE(TokenKind::TokenName.is_keyword());             \
     EXPECT_EQ(Spelling, TokenKind::TokenName.fixed_spelling()); \

toolchain/lexer/tokenized_buffer.cpp

@@ -652,30 +652,26 @@ auto TokenizedBuffer::GetTokenText(Token token) const -> llvm::StringRef {
     return llvm::StringRef();
   }
 
-  CARBON_CHECK(token_info.kind == TokenKind::Identifier)
-      << token_info.kind.name();
+  CARBON_CHECK(token_info.kind == TokenKind::Identifier) << token_info.kind;
   return GetIdentifierText(token_info.id);
 }
 
 auto TokenizedBuffer::GetIdentifier(Token token) const -> Identifier {
   const auto& token_info = GetTokenInfo(token);
-  CARBON_CHECK(token_info.kind == TokenKind::Identifier)
-      << token_info.kind.name();
+  CARBON_CHECK(token_info.kind == TokenKind::Identifier) << token_info.kind;
   return token_info.id;
 }
 
 auto TokenizedBuffer::GetIntegerLiteral(Token token) const
     -> const llvm::APInt& {
   const auto& token_info = GetTokenInfo(token);
-  CARBON_CHECK(token_info.kind == TokenKind::IntegerLiteral)
-      << token_info.kind.name();
+  CARBON_CHECK(token_info.kind == TokenKind::IntegerLiteral) << token_info.kind;
   return literal_int_storage_[token_info.literal_index];
 }
 
 auto TokenizedBuffer::GetRealLiteral(Token token) const -> RealLiteralValue {
   const auto& token_info = GetTokenInfo(token);
-  CARBON_CHECK(token_info.kind == TokenKind::RealLiteral)
-      << token_info.kind.name();
+  CARBON_CHECK(token_info.kind == TokenKind::RealLiteral) << token_info.kind;
 
   // Note that every real literal is at least three characters long, so we can
   // safely look at the second character to determine whether we have a
@@ -690,16 +686,14 @@ auto TokenizedBuffer::GetRealLiteral(Token token) const -> RealLiteralValue {
 
 auto TokenizedBuffer::GetStringLiteral(Token token) const -> llvm::StringRef {
   const auto& token_info = GetTokenInfo(token);
-  CARBON_CHECK(token_info.kind == TokenKind::StringLiteral)
-      << token_info.kind.name();
+  CARBON_CHECK(token_info.kind == TokenKind::StringLiteral) << token_info.kind;
   return literal_string_storage_[token_info.literal_index];
 }
 
 auto TokenizedBuffer::GetTypeLiteralSize(Token token) const
     -> const llvm::APInt& {
   const auto& token_info = GetTokenInfo(token);
-  CARBON_CHECK(token_info.kind.is_sized_type_literal())
-      << token_info.kind.name();
+  CARBON_CHECK(token_info.kind.is_sized_type_literal()) << token_info.kind;
   return literal_int_storage_[token_info.literal_index];
 }
 
@@ -707,7 +701,7 @@ auto TokenizedBuffer::GetMatchedClosingToken(Token opening_token) const
     -> Token {
   const auto& opening_token_info = GetTokenInfo(opening_token);
   CARBON_CHECK(opening_token_info.kind.is_opening_symbol())
-      << opening_token_info.kind.name();
+      << opening_token_info.kind;
   return opening_token_info.closing_token;
 }
 
@@ -715,7 +709,7 @@ auto TokenizedBuffer::GetMatchedOpeningToken(Token closing_token) const
     -> Token {
   const auto& closing_token_info = GetTokenInfo(closing_token);
   CARBON_CHECK(closing_token_info.kind.is_closing_symbol())
-      << closing_token_info.kind.name();
+      << closing_token_info.kind;
   return closing_token_info.opening_token;
 }
 

toolchain/lexer/tokenized_buffer_test_helpers.h

@@ -28,7 +28,7 @@ namespace Testing {
 struct ExpectedToken {
   friend auto operator<<(std::ostream& output, const ExpectedToken& expected)
       -> std::ostream& {
-    output << "\ntoken: { kind: '" << expected.kind.name().str() << "'";
+    output << "\ntoken: { kind: '" << expected.kind << "'";
     if (expected.line != -1) {
       output << ", line: " << expected.line;
     }
@@ -82,9 +82,8 @@ MATCHER_P(HasTokens, raw_all_expected, "") {
 
     TokenKind actual_kind = buffer.GetKind(token);
     if (actual_kind != expected.kind) {
-      *result_listener << "\nToken " << index << " is a "
-                       << actual_kind.name().str() << ", expected a "
-                       << expected.kind.name().str() << ".";
+      *result_listener << "\nToken " << index << " is a " << actual_kind
+                       << ", expected a " << expected.kind << ".";
       matches = false;
     }
 

toolchain/parser/parse_tree.cpp

@@ -95,7 +95,7 @@ auto ParseTree::PrintNode(llvm::raw_ostream& output, Node n, int depth,
   if (preorder) {
     output << "node_index: " << n << ", ";
   }
-  output << "kind: '" << n_impl.kind.name() << "', text: '"
+  output << "kind: '" << n_impl.kind << "', text: '"
          << tokens_->GetTokenText(n_impl.token) << "'";
 
   if (n_impl.has_error) {

toolchain/parser/parser.cpp

@@ -77,8 +77,8 @@ class Parser::PrettyStackTraceParseState : public llvm::PrettyStackTraceEntry {
     auto line = parser_->tokens_->GetLine(token);
     output << " @ " << parser_->tokens_->GetLineNumber(line) << ":"
            << parser_->tokens_->GetColumnNumber(token) << ":"
-           << " token " << token << " : "
-           << parser_->tokens_->GetKind(token).name() << "\n";
+           << " token " << token << " : " << parser_->tokens_->GetKind(token)
+           << "\n";
   }
 
   const Parser* parser_;
@@ -97,7 +97,7 @@ Parser::Parser(ParseTree& tree, TokenizedBuffer& tokens,
   --end_;
   CARBON_CHECK(tokens_->GetKind(*end_) == TokenKind::EndOfFile)
       << "TokenizedBuffer should end with EndOfFile, ended with "
-      << tokens_->GetKind(*end_).name();
+      << tokens_->GetKind(*end_);
 }
 
 auto Parser::AddLeafNode(ParseNodeKind kind, TokenizedBuffer::Token token,
@@ -164,7 +164,7 @@ auto Parser::ConsumeAndAddLeafNodeIf(TokenKind token_kind,
 
 auto Parser::ConsumeChecked(TokenKind kind) -> TokenizedBuffer::Token {
   CARBON_CHECK(PositionIs(kind))
-      << "Required " << kind.name() << ", found " << PositionKind().name();
+      << "Required " << kind << ", found " << PositionKind();
   return Consume();
 }
 

toolchain/semantics/semantics_parse_tree_handler.cpp

@@ -350,7 +350,7 @@ auto SemanticsParseTreeHandler::HandleInfixOperator(ParseTree::Node parse_node)
                                      parse_node, result_type, lhs_id, rhs_id));
       break;
     default:
-      CARBON_FATAL() << "Unrecognized token kind: " << token_kind.name();
+      CARBON_FATAL() << "Unrecognized token kind: " << token_kind;
   }
 }
 
@@ -393,7 +393,7 @@ auto SemanticsParseTreeHandler::HandleLiteral(ParseTree::Node parse_node)
       break;
     }
     default:
-      CARBON_FATAL() << "Unhandled kind: " << token_kind.name();
+      CARBON_FATAL() << "Unhandled kind: " << token_kind;
   }
 }