From 9fcbad8a42fdeefe4654c9d51cc18c62bab92166 Mon Sep 17 00:00:00 2001
From: Richard Smith <richard@metafoo.co.uk>
Date: Fri, 21 Oct 2022 08:02:01 -0700
Subject: [PATCH] Add factory function for empty source location. (#2328)

This is intended to be used only when creating source locations that are known to be ignored because they are fed into operations whose diagnostics are discarded and that do not store the location in any created object.

As requested in review of #2321.
---
 explorer/common/source_location.h     |  7 +++++++
 explorer/interpreter/type_checker.cpp | 20 ++++++++------------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/explorer/common/source_location.h b/explorer/common/source_location.h
index 2e11e67dcfd1..1b35a3602dcd 100644
--- a/explorer/common/source_location.h
+++ b/explorer/common/source_location.h
@@ -15,6 +15,13 @@ namespace Carbon {
 
 class SourceLocation {
  public:
+  // Produce a source location that is known to not be used, because it is fed
+  // into an operation that creates no AST nodes and whose diagnostics are
+  // discarded.
+  static auto DiagnosticsIgnored() -> SourceLocation {
+    return SourceLocation("", 0);
+  }
+
   // The filename should be eternal or arena-allocated to eliminate copies.
   constexpr SourceLocation(const char* filename, int line_num)
       : filename_(filename), line_num_(line_num) {}
diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp
index 258ee082bcae..a0a98d86b4ec 100644
--- a/explorer/interpreter/type_checker.cpp
+++ b/explorer/interpreter/type_checker.cpp
@@ -500,8 +500,7 @@ auto TypeChecker::IsImplicitlyConvertible(
   }
 
   // We didn't find a builtin implicit conversion. Try a user-defined one.
-  // The source location doesn't matter, we're discarding the diagnostics.
-  SourceLocation source_loc("", 0);
+  SourceLocation source_loc = SourceLocation::DiagnosticsIgnored();
   ErrorOr<Nonnull<const InterfaceType*>> iface_type = GetBuiltinInterfaceType(
       source_loc, BuiltinInterfaceName{Builtins::ImplicitAs, destination});
   return iface_type.ok() &&
@@ -1596,16 +1595,14 @@ auto TypeChecker::Substitute(const Bindings& bindings,
       }
       ConstraintTypeBuilder builder(arena_,
                                     constraint.self_binding()->source_loc());
-      // Diagnostics are discarded (except in CHECK failure message).
-      SourceLocation source_loc("", 0);
       ErrorOr<Success> result = builder.AddAndSubstitute(
-          *this, source_loc, &constraint, builder.GetSelfType(),
-          builder.GetSelfWitness(), bindings,
+          *this, SourceLocation::DiagnosticsIgnored(), &constraint,
+          builder.GetSelfType(), builder.GetSelfWitness(), bindings,
           /*add_lookup_contexts=*/true);
       // TODO: This appears to theoretically be possible, and should be handled
       // better.
       CARBON_CHECK(result.ok()) << "substitution into " << constraint
-                                << " failed: " << result.error();
+                                << " failed: " << result.error().message();
       Nonnull<const ConstraintType*> new_constraint =
           std::move(builder).Build();
       if (trace_stream_) {
@@ -1709,12 +1706,11 @@ auto TypeChecker::RefineWitness(Nonnull<const Witness*> witness,
     return witness;
   }
 
-  // No source location; diagnostics will be discarded.
-  SourceLocation source_loc("", 0);
-
   // Attempt to look for an impl witness in the top-level impl scope.
-  if (auto refined_witness = (*top_level_impl_scope_)
-                                 ->Resolve(constraint, type, source_loc, *this);
+  if (auto refined_witness =
+          (*top_level_impl_scope_)
+              ->Resolve(constraint, type, SourceLocation::DiagnosticsIgnored(),
+                        *this);
       refined_witness.ok()) {
     return *refined_witness;
   } else {