From 05723095bcfcf7dbfb59ad367e729f591c32e155 Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Thu, 30 Nov 2023 10:33:03 -0800
Subject: [PATCH] Switch formatv adapters to format_provider (#3434)

The main difference I'm aiming for is that clangd doesn't complain about
the struct being unused, but it does miss the function's use. But really
these are specific to formatv for diagnostics, so this is more clearly
marking such, and probably makes for a better pattern for the future.
---
 toolchain/lex/numeric_literal.cpp | 44 ++++++++++++++++++-------------
 toolchain/parse/context.cpp       | 40 +++++++++++++++++-----------
 2 files changed, 50 insertions(+), 34 deletions(-)

diff --git a/toolchain/lex/numeric_literal.cpp b/toolchain/lex/numeric_literal.cpp
index bac66dc409e2..51d582bf716b 100644
--- a/toolchain/lex/numeric_literal.cpp
+++ b/toolchain/lex/numeric_literal.cpp
@@ -8,30 +8,36 @@
 
 #include "common/check.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/FormatVariadicDetails.h"
 #include "toolchain/lex/character_set.h"
 #include "toolchain/lex/helpers.h"
 
-namespace Carbon::Lex {
+namespace llvm {
 
-// Adapts Radix for use with formatv.
-// NOTE: clangd may see this as unused, but it will be invoked by diagnostics.
-// We don't do anything to disable the warning because clang compile invocations
-// should warn if it's actually unused.
-static auto operator<<(llvm::raw_ostream& out, NumericLiteral::Radix radix)
-    -> llvm::raw_ostream& {
-  switch (radix) {
-    case NumericLiteral::Radix::Binary:
-      out << "binary";
-      break;
-    case NumericLiteral::Radix::Decimal:
-      out << "decimal";
-      break;
-    case NumericLiteral::Radix::Hexadecimal:
-      out << "hexadecimal";
-      break;
+// 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::Lex::NumericLiteral::Radix> {
+  using Radix = Carbon::Lex::NumericLiteral::Radix;
+  static void format(const Radix& radix, raw_ostream& out,
+                     StringRef /*style*/) {
+    switch (radix) {
+      case Radix::Binary:
+        out << "binary";
+        break;
+      case Radix::Decimal:
+        out << "decimal";
+        break;
+      case Radix::Hexadecimal:
+        out << "hexadecimal";
+        break;
+    }
   }
-  return out;
-}
+};
+
+}  // namespace llvm
+
+namespace Carbon::Lex {
 
 auto NumericLiteral::Lex(llvm::StringRef source_text)
     -> std::optional<NumericLiteral> {
diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp
index 720372204f2e..9983ddeb95b3 100644
--- a/toolchain/parse/context.cpp
+++ b/toolchain/parse/context.cpp
@@ -23,23 +23,33 @@ enum class RelativeLocation : int8_t {
   Before,
 };
 
+}  // namespace Carbon::Parse
+
+namespace llvm {
+
 // Adapts RelativeLocation for use with formatv.
-// TODO: Investigate for approaches that clangd is happier with.
-static auto operator<<(llvm::raw_ostream& out, RelativeLocation loc)
-    -> llvm::raw_ostream& {
-  switch (loc) {
-    case RelativeLocation::Around:
-      out << "around";
-      break;
-    case RelativeLocation::After:
-      out << "after";
-      break;
-    case RelativeLocation::Before:
-      out << "before";
-      break;
+template <>
+struct format_provider<Carbon::Parse::RelativeLocation> {
+  using RelativeLocation = Carbon::Parse::RelativeLocation;
+  static void format(const RelativeLocation& loc, raw_ostream& out,
+                     StringRef /*style*/) {
+    switch (loc) {
+      case RelativeLocation::Around:
+        out << "around";
+        break;
+      case RelativeLocation::After:
+        out << "after";
+        break;
+      case RelativeLocation::Before:
+        out << "before";
+        break;
+    }
   }
-  return out;
-}
+};
+
+}  // namespace llvm
+
+namespace Carbon::Parse {
 
 Context::Context(Tree& tree, Lex::TokenizedBuffer& tokens,
                  Lex::TokenDiagnosticEmitter& emitter,