Quellcode durchsuchen

Improve autoupdate diagnostic CHECK line positioning. (#5942)

When an error diagnostic has an unattached location, for example because
the diagnostic points into a file that's in the prelude, use the next
attached location to position the error diagnostic's CHECK line. In
particular, if the error is followed by a note, use the position of the
note to determine where to place the error.

This exposes a general mechanism to do final fixups of the CHECK lines
to individual file_test binaries, which the toolchain's binary uses to
special-case error / warning CHECK lines.
Richard Smith vor 8 Monaten
Ursprung
Commit
f616817b71

+ 7 - 3
testing/file_test/autoupdate.cpp

@@ -150,7 +150,7 @@ auto FileTestAutoupdater::GetFileAndLineNumber(
 }
 
 auto FileTestAutoupdater::BuildCheckLines(llvm::StringRef output,
-                                          const char* label) -> CheckLines {
+                                          bool is_stderr) -> CheckLines {
   if (output.empty()) {
     return CheckLines({});
   }
@@ -166,16 +166,19 @@ auto FileTestAutoupdater::BuildCheckLines(llvm::StringRef output,
     lines.pop_back();
   }
 
+  auto label =
+      is_stderr ? llvm::StringLiteral("STDERR") : llvm::StringLiteral("STDOUT");
+
   // The default file number for when no specific file is found.
   int default_file_number = 0;
 
-  llvm::SmallVector<CheckLine> check_lines;
+  CheckLineArray check_lines;
   for (const auto& line : lines) {
     // This code is relatively hot in our testing, and because when testing it
     // isn't run with an optimizer we benefit from making it use simple
     // constructs. For this reason, we avoid `llvm::formatv` and similar tools.
     std::string check_line;
-    check_line.reserve(line.size() + strlen(label) + strlen("// CHECK:: "));
+    check_line.reserve(line.size() + label.size() + strlen("// CHECK:: "));
     check_line.append("// CHECK:");
     check_line.append(label);
     check_line.append(":");
@@ -219,6 +222,7 @@ auto FileTestAutoupdater::BuildCheckLines(llvm::StringRef output,
                                               default_file_number, check_line);
     check_lines.push_back(CheckLine(file_and_line, check_line));
   }
+  finalize_check_lines_(check_lines, is_stderr);
   return CheckLines(check_lines);
 }
 

+ 67 - 61
testing/file_test/autoupdate.h

@@ -33,6 +33,64 @@ class FileTestAutoupdater {
     std::string line_formatv;
   };
 
+  // The file and line number that a CHECK line refers to, and the
+  // replacement from which they were determined, if any.
+  struct FileAndLineNumber {
+    explicit FileAndLineNumber(int file_number) : file_number(file_number) {}
+
+    explicit FileAndLineNumber(const LineNumberReplacement* replacement,
+                               int file_number, absl::string_view line_number);
+
+    const LineNumberReplacement* replacement = nullptr;
+    int file_number;
+    int line_number = -1;
+  };
+
+  // A CHECK line which is integrated into autoupdate output.
+  //
+  // `final` because we use pointer arithmetic on this type.
+  class CheckLine final : public FileTestLineBase {
+   public:
+    explicit CheckLine(FileAndLineNumber file_and_line_number, std::string line)
+        : FileTestLineBase(file_and_line_number.file_number,
+                           file_and_line_number.line_number),
+          replacement_(file_and_line_number.replacement),
+          line_(std::move(line)) {}
+
+    auto Print(llvm::raw_ostream& out) const -> void override {
+      out << indent_ << line_;
+    }
+
+    // When the location of the CHECK in output is known, we can set the indent
+    // and its line.
+    auto SetOutputLine(llvm::StringRef indent, int output_file_number,
+                       int output_line_number) -> void {
+      indent_ = indent;
+      output_file_number_ = output_file_number;
+      output_line_number_ = output_line_number;
+    }
+
+    // When the location of all lines in a file are known, we can set the line
+    // offset based on the target line.
+    auto RemapLineNumbers(
+        const llvm::DenseMap<llvm::StringRef, int>& file_to_number_map,
+        const llvm::DenseMap<std::pair<int, int>, int>& output_line_remap,
+        const llvm::SmallVector<int>& new_last_line_numbers) -> void;
+
+    auto line() const -> llvm::StringRef { return line_; }
+
+    auto is_blank() const -> bool override { return false; }
+
+   private:
+    const LineNumberReplacement* replacement_;
+    std::string line_;
+    llvm::StringRef indent_;
+    int output_file_number_ = -1;
+    int output_line_number_ = -1;
+  };
+
+  using CheckLineArray = llvm::SmallVector<FileTestAutoupdater::CheckLine>;
+
   explicit FileTestAutoupdater(
       const std::filesystem::path& file_test_path, std::string test_command,
       std::string dump_command, llvm::StringRef input_content,
@@ -42,7 +100,8 @@ class FileTestAutoupdater {
       llvm::StringRef actual_stdout, llvm::StringRef actual_stderr,
       const std::optional<RE2>& default_file_re,
       const llvm::SmallVector<LineNumberReplacement>& line_number_replacements,
-      std::function<auto(std::string&)->void> do_extra_check_replacements)
+      std::function<auto(std::string&)->void> do_extra_check_replacements,
+      std::function<auto(CheckLineArray&, bool)->void> finalize_check_lines)
       : file_test_path_(file_test_path),
         test_command_(std::move(test_command)),
         dump_command_(std::move(dump_command)),
@@ -52,13 +111,14 @@ class FileTestAutoupdater {
         default_file_re_(default_file_re),
         line_number_replacements_(line_number_replacements),
         do_extra_check_replacements_(std::move(do_extra_check_replacements)),
+        finalize_check_lines_(std::move(finalize_check_lines)),
         autoupdate_split_file_(
             autoupdate_split ? std::optional(filenames.size()) : std::nullopt),
         file_to_number_map_(BuildFileToNumberMap(filenames)),
         // BuildCheckLines should only be called after other member
         // initialization.
-        stdout_(BuildCheckLines(actual_stdout, "STDOUT")),
-        stderr_(BuildCheckLines(actual_stderr, "STDERR")),
+        stdout_(BuildCheckLines(actual_stdout, /*is_stderr=*/false)),
+        stderr_(BuildCheckLines(actual_stderr, /*is_stderr=*/true)),
         any_attached_stdout_lines_(llvm::any_of(
             stdout_.lines,
             [&](const CheckLine& line) { return line.line_number() != -1; })),
@@ -78,19 +138,6 @@ class FileTestAutoupdater {
   auto Run(bool dry_run) -> bool;
 
  private:
-  // The file and line number that a CHECK line refers to, and the
-  // replacement from which they were determined, if any.
-  struct FileAndLineNumber {
-    explicit FileAndLineNumber(int file_number) : file_number(file_number) {}
-
-    explicit FileAndLineNumber(const LineNumberReplacement* replacement,
-                               int file_number, absl::string_view line_number);
-
-    const LineNumberReplacement* replacement = nullptr;
-    int file_number;
-    int line_number = -1;
-  };
-
   // A TIP line added by autoupdate. Not associated with any line in output.
   class TipLine : public FileTestLineBase {
    public:
@@ -105,55 +152,13 @@ class FileTestAutoupdater {
     std::string line_;
   };
 
-  // A CHECK line which is integrated into autoupdate output.
-  //
-  // `final` because we use pointer arithmetic on this type.
-  class CheckLine final : public FileTestLineBase {
-   public:
-    // RE2 is passed by a pointer because it doesn't support std::optional.
-    explicit CheckLine(FileAndLineNumber file_and_line_number, std::string line)
-        : FileTestLineBase(file_and_line_number.file_number,
-                           file_and_line_number.line_number),
-          replacement_(file_and_line_number.replacement),
-          line_(std::move(line)) {}
-
-    auto Print(llvm::raw_ostream& out) const -> void override {
-      out << indent_ << line_;
-    }
-
-    // When the location of the CHECK in output is known, we can set the indent
-    // and its line.
-    auto SetOutputLine(llvm::StringRef indent, int output_file_number,
-                       int output_line_number) -> void {
-      indent_ = indent;
-      output_file_number_ = output_file_number;
-      output_line_number_ = output_line_number;
-    }
-
-    // When the location of all lines in a file are known, we can set the line
-    // offset based on the target line.
-    auto RemapLineNumbers(
-        const llvm::DenseMap<llvm::StringRef, int>& file_to_number_map,
-        const llvm::DenseMap<std::pair<int, int>, int>& output_line_remap,
-        const llvm::SmallVector<int>& new_last_line_numbers) -> void;
-
-    auto is_blank() const -> bool override { return false; }
-
-   private:
-    const LineNumberReplacement* replacement_;
-    std::string line_;
-    llvm::StringRef indent_;
-    int output_file_number_ = -1;
-    int output_line_number_ = -1;
-  };
-
   // Clusters information for stdout and stderr.
   struct CheckLines {
-    explicit CheckLines(llvm::SmallVector<CheckLine> lines)
+    explicit CheckLines(CheckLineArray lines)
         : lines(std::move(lines)), cursor(this->lines.begin()) {}
 
     // The full list of check lines.
-    llvm::SmallVector<CheckLine> lines;
+    CheckLineArray lines;
     // An iterator into check_lines.
     CheckLine* cursor;
   };
@@ -176,7 +181,7 @@ class FileTestAutoupdater {
   }
 
   // Builds CheckLine lists for autoupdate.
-  auto BuildCheckLines(llvm::StringRef output, const char* label) -> CheckLines;
+  auto BuildCheckLines(llvm::StringRef output, bool is_stderr) -> CheckLines;
 
   // Adds a non-check line to the new_lines and output_line_remap. The caller
   // still needs to advance the cursor when ready.
@@ -214,6 +219,7 @@ class FileTestAutoupdater {
   const std::optional<RE2>& default_file_re_;
   const llvm::SmallVector<LineNumberReplacement>& line_number_replacements_;
   std::function<auto(std::string&)->void> do_extra_check_replacements_;
+  std::function<auto(CheckLineArray&, bool)->void> finalize_check_lines_;
 
   // If we have an autoupdate split that still needs to be processed, the file
   // number of the autoupdate split. Otherwise, this is nullopt.

+ 3 - 0
testing/file_test/file_test_base.cpp

@@ -194,6 +194,9 @@ static auto RunAutoupdater(FileTestBase* test_base, const TestFile& test_file,
              test_base->GetLineNumberReplacements(expected_filenames),
              [&](std::string& line) {
                test_base->DoExtraCheckReplacements(line);
+             },
+             [&](FileTestBase::CheckLineArray& lines, bool is_stderr) {
+               test_base->FinalizeCheckLines(lines, is_stderr);
              })
       .Run(dry_run);
 }

+ 6 - 0
testing/file_test/file_test_base.h

@@ -28,6 +28,7 @@ class FileTestBase {
  public:
   // Provided for child class convenience.
   using LineNumberReplacement = FileTestAutoupdater::LineNumberReplacement;
+  using CheckLineArray = FileTestAutoupdater::CheckLineArray;
 
   // The result of Run(), used to detect errors. Failing test files should be
   // named with a `fail_` prefix to indicate an expectation of failure.
@@ -101,6 +102,11 @@ class FileTestBase {
   virtual auto DoExtraCheckReplacements(std::string& /*check_line*/) const
       -> void {}
 
+  // Optionally allows children to perform tweaks on check lines before
+  // they are merged into the output file.
+  virtual auto FinalizeCheckLines(CheckLineArray& /*check_lines*/,
+                                  bool /*is_stderr*/) const -> void {}
+
   // Whether to allow running the test in parallel, particularly for autoupdate.
   // This can be overridden to force some tests to be run serially. At any given
   // time, all parallel tests and a single non-parallel test will be allowed to

+ 5 - 0
testing/file_test/line.h

@@ -25,6 +25,11 @@ class FileTestLineBase : public Printable<FileTestLineBase> {
   auto file_number() const -> int { return file_number_; }
   auto line_number() const -> int { return line_number_; }
 
+  void set_location(int file_number, int line_number) {
+    file_number_ = file_number;
+    line_number_ = line_number;
+  }
+
  private:
   int file_number_;
   int line_number_;

+ 4 - 4
toolchain/check/testdata/basics/type_literals.carbon

@@ -78,14 +78,14 @@ var test_i0: i0;
 // CHECK:STDERR:              ^~
 // CHECK:STDERR:
 var test_i1: i1;
-// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+7]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.Int(15)` instead [IntWidthNotMultipleOf8]
+// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+4]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.Int(15)` instead [IntWidthNotMultipleOf8]
 // CHECK:STDERR: var test_i15: i15;
 // CHECK:STDERR:               ^~~
 // CHECK:STDERR:
+var test_i15: i15;
 // CHECK:STDERR: min_prelude/parts/int.carbon:10:9: error: integer type width of 1000000000 is greater than the maximum supported width of 8388608 [IntWidthTooLarge]
 // CHECK:STDERR:   adapt MakeInt(N);
 // CHECK:STDERR:         ^~~~~~~~~~
-var test_i15: i15;
 // CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+4]]:23: note: in `i1000000000` used here [ResolvingSpecificHere]
 // CHECK:STDERR: var test_i1000000000: i1000000000;
 // CHECK:STDERR:                       ^~~~~~~~~~~
@@ -115,14 +115,14 @@ var test_u0: u0;
 // CHECK:STDERR:              ^~
 // CHECK:STDERR:
 var test_u1: u1;
-// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+7]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.UInt(15)` instead [IntWidthNotMultipleOf8]
+// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+4]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.UInt(15)` instead [IntWidthNotMultipleOf8]
 // CHECK:STDERR: var test_u15: u15;
 // CHECK:STDERR:               ^~~
 // CHECK:STDERR:
+var test_u15: u15;
 // CHECK:STDERR: min_prelude/parts/uint.carbon:10:9: error: integer type width of 1000000000 is greater than the maximum supported width of 8388608 [IntWidthTooLarge]
 // CHECK:STDERR:   adapt MakeUInt(N);
 // CHECK:STDERR:         ^~~~~~~~~~~
-var test_u15: u15;
 // CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+4]]:23: note: in `u1000000000` used here [ResolvingSpecificHere]
 // CHECK:STDERR: var test_u1000000000: u1000000000;
 // CHECK:STDERR:                       ^~~~~~~~~~~

+ 23 - 23
toolchain/check/testdata/function/generic/resolve_used.carbon

@@ -11,9 +11,6 @@
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/function/generic/resolve_used.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/function/generic/resolve_used.carbon
-// CHECK:STDERR: min_prelude/parts/int.carbon:10:9: error: integer type width of 0 is not positive [IntWidthNotPositive]
-// CHECK:STDERR:   adapt MakeInt(N);
-// CHECK:STDERR:         ^~~~~~~~~~
 
 // --- fail_todo_call_monomorphization_error.carbon
 
@@ -24,6 +21,9 @@ fn ErrorIfNIsZero(N:! Core.IntLiteral()) {
   // ensuring we produce an error when doing so. Notionally this error is
   // produced as a result of instantiating the `Core.Int` template, although
   // that's not how we currently model `Core.Int`.
+  // CHECK:STDERR: min_prelude/parts/int.carbon:10:9: error: integer type width of 0 is not positive [IntWidthNotPositive]
+  // CHECK:STDERR:   adapt MakeInt(N);
+  // CHECK:STDERR:         ^~~~~~~~~~
   // CHECK:STDERR: fail_todo_call_monomorphization_error.carbon:[[@LINE+3]]:10: note: in `i0` used here [ResolvingSpecificHere]
   // CHECK:STDERR:   var v: Core.Int(N);
   // CHECK:STDERR:          ^~~~~~~~~~~
@@ -121,17 +121,17 @@ fn CallNegative() {
 // CHECK:STDOUT:   %N.loc4_19.1: Core.IntLiteral = bind_symbolic_name N, 0 [symbolic = %N.loc4_19.1 (constants.%N)]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
-// CHECK:STDOUT:   %Int.loc12_20.2: type = class_type @Int, @Int(%N.loc4_19.1) [symbolic = %Int.loc12_20.2 (constants.%Int.49d0e6.1)]
-// CHECK:STDOUT:   %require_complete.loc12_20: <witness> = require_complete_type %Int.loc12_20.2 [symbolic = %require_complete.loc12_20 (constants.%require_complete.b4f426.1)]
-// CHECK:STDOUT:   %pattern_type: type = pattern_type %Int.loc12_20.2 [symbolic = %pattern_type (constants.%pattern_type.8963eb.1)]
+// CHECK:STDOUT:   %Int.loc15_20.2: type = class_type @Int, @Int(%N.loc4_19.1) [symbolic = %Int.loc15_20.2 (constants.%Int.49d0e6.1)]
+// CHECK:STDOUT:   %require_complete.loc15_20: <witness> = require_complete_type %Int.loc15_20.2 [symbolic = %require_complete.loc15_20 (constants.%require_complete.b4f426.1)]
+// CHECK:STDOUT:   %pattern_type: type = pattern_type %Int.loc15_20.2 [symbolic = %pattern_type (constants.%pattern_type.8963eb.1)]
 // CHECK:STDOUT:   %Destroy.impl_witness: <witness> = impl_witness imports.%Destroy.impl_witness_table.72b, @Int.as.Destroy.impl(%N.loc4_19.1) [symbolic = %Destroy.impl_witness (constants.%Destroy.impl_witness.543)]
-// CHECK:STDOUT:   %Destroy.facet: %Destroy.type = facet_value %Int.loc12_20.2, (%Destroy.impl_witness) [symbolic = %Destroy.facet (constants.%Destroy.facet.abb)]
-// CHECK:STDOUT:   %.loc12_3: type = fn_type_with_self_type constants.%Destroy.Op.type, %Destroy.facet [symbolic = %.loc12_3 (constants.%.94a)]
+// CHECK:STDOUT:   %Destroy.facet: %Destroy.type = facet_value %Int.loc15_20.2, (%Destroy.impl_witness) [symbolic = %Destroy.facet (constants.%Destroy.facet.abb)]
+// CHECK:STDOUT:   %.loc15_3: type = fn_type_with_self_type constants.%Destroy.Op.type, %Destroy.facet [symbolic = %.loc15_3 (constants.%.94a)]
 // CHECK:STDOUT:   %Int.as.Destroy.impl.Op.type: type = fn_type @Int.as.Destroy.impl.Op, @Int.as.Destroy.impl(%N.loc4_19.1) [symbolic = %Int.as.Destroy.impl.Op.type (constants.%Int.as.Destroy.impl.Op.type.733)]
 // CHECK:STDOUT:   %Int.as.Destroy.impl.Op: @ErrorIfNIsZero.%Int.as.Destroy.impl.Op.type (%Int.as.Destroy.impl.Op.type.733) = struct_value () [symbolic = %Int.as.Destroy.impl.Op (constants.%Int.as.Destroy.impl.Op.011)]
 // CHECK:STDOUT:   %Int.as.Destroy.impl.Op.specific_fn: <specific function> = specific_function %Int.as.Destroy.impl.Op, @Int.as.Destroy.impl.Op(%N.loc4_19.1) [symbolic = %Int.as.Destroy.impl.Op.specific_fn (constants.%Int.as.Destroy.impl.Op.specific_fn.1f5)]
-// CHECK:STDOUT:   %ptr: type = ptr_type %Int.loc12_20.2 [symbolic = %ptr (constants.%ptr.784)]
-// CHECK:STDOUT:   %require_complete.loc12_3: <witness> = require_complete_type %ptr [symbolic = %require_complete.loc12_3 (constants.%require_complete.0f5)]
+// CHECK:STDOUT:   %ptr: type = ptr_type %Int.loc15_20.2 [symbolic = %ptr (constants.%ptr.784)]
+// CHECK:STDOUT:   %require_complete.loc15_3: <witness> = require_complete_type %ptr [symbolic = %require_complete.loc15_3 (constants.%require_complete.0f5)]
 // CHECK:STDOUT:
 // CHECK:STDOUT:   fn() {
 // CHECK:STDOUT:   !entry:
@@ -139,20 +139,20 @@ fn CallNegative() {
 // CHECK:STDOUT:       %v.patt: @ErrorIfNIsZero.%pattern_type (%pattern_type.8963eb.1) = binding_pattern v [concrete]
 // CHECK:STDOUT:       %v.var_patt: @ErrorIfNIsZero.%pattern_type (%pattern_type.8963eb.1) = var_pattern %v.patt [concrete]
 // CHECK:STDOUT:     }
-// CHECK:STDOUT:     %v.var: ref @ErrorIfNIsZero.%Int.loc12_20.2 (%Int.49d0e6.1) = var %v.var_patt
-// CHECK:STDOUT:     %.loc12_20: type = splice_block %Int.loc12_20.1 [symbolic = %Int.loc12_20.2 (constants.%Int.49d0e6.1)] {
-// CHECK:STDOUT:       %Core.ref.loc12: <namespace> = name_ref Core, imports.%Core [concrete = imports.%Core]
+// CHECK:STDOUT:     %v.var: ref @ErrorIfNIsZero.%Int.loc15_20.2 (%Int.49d0e6.1) = var %v.var_patt
+// CHECK:STDOUT:     %.loc15_20: type = splice_block %Int.loc15_20.1 [symbolic = %Int.loc15_20.2 (constants.%Int.49d0e6.1)] {
+// CHECK:STDOUT:       %Core.ref.loc15: <namespace> = name_ref Core, imports.%Core [concrete = imports.%Core]
 // CHECK:STDOUT:       %Int.ref: %Int.type = name_ref Int, imports.%Core.Int [concrete = constants.%Int.generic]
 // CHECK:STDOUT:       %N.ref: Core.IntLiteral = name_ref N, %N.loc4_19.2 [symbolic = %N.loc4_19.1 (constants.%N)]
-// CHECK:STDOUT:       %Int.loc12_20.1: type = class_type @Int, @Int(constants.%N) [symbolic = %Int.loc12_20.2 (constants.%Int.49d0e6.1)]
+// CHECK:STDOUT:       %Int.loc15_20.1: type = class_type @Int, @Int(constants.%N) [symbolic = %Int.loc15_20.2 (constants.%Int.49d0e6.1)]
 // CHECK:STDOUT:     }
-// CHECK:STDOUT:     %v: ref @ErrorIfNIsZero.%Int.loc12_20.2 (%Int.49d0e6.1) = bind_name v, %v.var
-// CHECK:STDOUT:     %impl.elem0: @ErrorIfNIsZero.%.loc12_3 (%.94a) = impl_witness_access constants.%Destroy.impl_witness.543, element0 [symbolic = %Int.as.Destroy.impl.Op (constants.%Int.as.Destroy.impl.Op.011)]
-// CHECK:STDOUT:     %bound_method.loc12_3.1: <bound method> = bound_method %v.var, %impl.elem0
+// CHECK:STDOUT:     %v: ref @ErrorIfNIsZero.%Int.loc15_20.2 (%Int.49d0e6.1) = bind_name v, %v.var
+// CHECK:STDOUT:     %impl.elem0: @ErrorIfNIsZero.%.loc15_3 (%.94a) = impl_witness_access constants.%Destroy.impl_witness.543, element0 [symbolic = %Int.as.Destroy.impl.Op (constants.%Int.as.Destroy.impl.Op.011)]
+// CHECK:STDOUT:     %bound_method.loc15_3.1: <bound method> = bound_method %v.var, %impl.elem0
 // CHECK:STDOUT:     %specific_fn: <specific function> = specific_function %impl.elem0, @Int.as.Destroy.impl.Op(constants.%N) [symbolic = %Int.as.Destroy.impl.Op.specific_fn (constants.%Int.as.Destroy.impl.Op.specific_fn.1f5)]
-// CHECK:STDOUT:     %bound_method.loc12_3.2: <bound method> = bound_method %v.var, %specific_fn
+// CHECK:STDOUT:     %bound_method.loc15_3.2: <bound method> = bound_method %v.var, %specific_fn
 // CHECK:STDOUT:     %addr: @ErrorIfNIsZero.%ptr (%ptr.784) = addr_of %v.var
-// CHECK:STDOUT:     %Int.as.Destroy.impl.Op.call: init %empty_tuple.type = call %bound_method.loc12_3.2(%addr)
+// CHECK:STDOUT:     %Int.as.Destroy.impl.Op.call: init %empty_tuple.type = call %bound_method.loc15_3.2(%addr)
 // CHECK:STDOUT:     return
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
@@ -174,16 +174,16 @@ fn CallNegative() {
 // CHECK:STDOUT:   %N.loc4_19.1 => constants.%int_0
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
-// CHECK:STDOUT:   %Int.loc12_20.2 => constants.%i0
-// CHECK:STDOUT:   %require_complete.loc12_20 => constants.%complete_type.d94
+// CHECK:STDOUT:   %Int.loc15_20.2 => constants.%i0
+// CHECK:STDOUT:   %require_complete.loc15_20 => constants.%complete_type.d94
 // CHECK:STDOUT:   %pattern_type => constants.%pattern_type.47b
 // CHECK:STDOUT:   %Destroy.impl_witness => constants.%Destroy.impl_witness.951
 // CHECK:STDOUT:   %Destroy.facet => constants.%Destroy.facet.4e0
-// CHECK:STDOUT:   %.loc12_3 => constants.%.53d
+// CHECK:STDOUT:   %.loc15_3 => constants.%.53d
 // CHECK:STDOUT:   %Int.as.Destroy.impl.Op.type => constants.%Int.as.Destroy.impl.Op.type.be2
 // CHECK:STDOUT:   %Int.as.Destroy.impl.Op => constants.%Int.as.Destroy.impl.Op.a22
 // CHECK:STDOUT:   %Int.as.Destroy.impl.Op.specific_fn => constants.%Int.as.Destroy.impl.Op.specific_fn.f1a
 // CHECK:STDOUT:   %ptr => constants.%ptr.3f1
-// CHECK:STDOUT:   %require_complete.loc12_3 => constants.%complete_type.588
+// CHECK:STDOUT:   %require_complete.loc15_3 => constants.%complete_type.588
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 1 - 0
toolchain/testing/BUILD

@@ -69,6 +69,7 @@ file_test(
     deps = [
         "//common:all_llvm_targets",
         "//common:error",
+        "//testing/file_test:autoupdate",
         "//testing/file_test:file_test_base",
         "//toolchain/driver",
         "@abseil-cpp//absl/strings",

+ 25 - 0
toolchain/testing/file_test.cpp

@@ -18,6 +18,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "testing/file_test/autoupdate.h"
 #include "testing/file_test/file_test_base.h"
 #include "toolchain/driver/driver.h"
 
@@ -103,6 +104,10 @@ class ToolchainFileTest : public FileTestBase {
   // driver.
   auto DoExtraCheckReplacements(std::string& check_line) const -> void override;
 
+  // Do some final tweaks to check line locations.
+  auto FinalizeCheckLines(CheckLineArray& check_lines, bool is_stderr) const
+      -> void override;
+
   // Most tests can be run in parallel, but clangd has a global for its logging
   // system so we need language-server tests to be run in serial.
   auto AllowParallelRun() const -> bool override {
@@ -320,6 +325,26 @@ auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) const
   }
 }
 
+auto ToolchainFileTest::FinalizeCheckLines(CheckLineArray& check_lines,
+                                           bool is_stderr) const -> void {
+  if (is_stderr) {
+    static const RE2 is_new_diagnostic_re(R"(.*:\d*:\d*: (error|warning): )");
+    // If a diagnostic isn't attached to a line, try to position it with its
+    // first note.
+    FileTestAutoupdater::CheckLine* diagnostic_without_loc = nullptr;
+    for (auto& check_line : check_lines) {
+      bool has_loc = check_line.line_number() != -1;
+      if (RE2::PartialMatch(check_line.line(), is_new_diagnostic_re)) {
+        diagnostic_without_loc = has_loc ? nullptr : &check_line;
+      } else if (has_loc && diagnostic_without_loc) {
+        diagnostic_without_loc->set_location(check_line.file_number(),
+                                             check_line.line_number());
+        diagnostic_without_loc = nullptr;
+      }
+    }
+  }
+}
+
 }  // namespace Carbon::Testing
 
 #endif  // CARBON_TOOLCHAIN_DRIVER_DRIVER_FILE_TEST_BASE_H_