Skip to content

Commit 7befe2c

Browse files
jonmeowchandlerc
andauthored
Switch custom error stream output to diagnostic (#4846)
This switches most error printing to use diagnostics instead of direct stream writes, even when not a specific file diagnostic. I'm allowing empty filenames for this use-case. This allows a little more specific testing to validate coverage of output using the diagnostic coverage test. I'm adding a few tests to cover things that weren't previously tested. Separately, this also forces a little more standardization in format... considering how changes like #4568 show effort being spent to _mirror_ diagnostic style, my thought is now to just use diagnostic code where possible. Note this also allows incrementally better testing of the language server; I'm changing the crash fix from #4847 in favor of diagnostic testing. --------- Co-authored-by: Chandler Carruth <[email protected]>
1 parent 7d2958a commit 7befe2c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+379
-174
lines changed

toolchain/check/check_unit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ auto CheckUnit::ProcessNodeIds() -> bool {
366366
auto converted = unit_and_imports_->unit->node_converter->ConvertLoc(
367367
node_id, [](DiagnosticLoc, const DiagnosticBase<>&) {});
368368
converted.loc.FormatLocation(output);
369-
output << ": checking " << context_.parse_tree().node_kind(node_id) << "\n";
369+
output << "checking " << context_.parse_tree().node_kind(node_id) << "\n";
370370
// Crash output has a tab indent; try to indent slightly past that.
371371
converted.loc.FormatSnippet(output, /*indent=*/10);
372372
});

toolchain/check/context.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def)
223223
.Emit();
224224
}
225225

226-
auto Context::DiagnosePoisonedName(SemIRLoc loc) -> void {
226+
auto Context::DiagnosePoisonedName(SemIR::InstId loc) -> void {
227227
// TODO: Improve the diagnostic to replace NodeId::None with the location
228228
// where the name was poisoned. See discussion in
229229
// https://github.com/carbon-language/carbon-lang/pull/4654#discussion_r1876607172

toolchain/check/context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ class Context {
266266
auto DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def) -> void;
267267

268268
// Prints a diagnostic for a poisoned name.
269-
auto DiagnosePoisonedName(SemIRLoc loc) -> void;
269+
auto DiagnosePoisonedName(SemIR::InstId loc) -> void;
270270

271271
// Prints a diagnostic for a missing name.
272272
auto DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id) -> void;

toolchain/check/testdata/basics/builtin_insts.carbon

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Exceptions. See /LICENSE for license information.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44
//
5-
// ARGS: compile --no-prelude-import --phase=check --dump-raw-sem-ir --builtin-sem-ir %s
5+
// ARGS: --include-diagnostic-kind compile --no-prelude-import --phase=check --dump-raw-sem-ir --builtin-sem-ir %s
66
//
77
// AUTOUPDATE
88
// TIP: To test this file alone, run:
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
2+
// Exceptions. See /LICENSE for license information.
3+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
4+
//
5+
// ARGS: --include-diagnostic-kind --fuzzing compile --no-prelude-import --phase=check %s
6+
//
7+
// AUTOUPDATE
8+
// TIP: To test this file alone, run:
9+
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon
10+
// TIP: To dump output, run:
11+
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/no_prelude/fail_fuzzing.carbon
12+
13+
// CHECK:STDERR: fail_fuzzing.carbon:[[@LINE+4]]:1: error: `Cpp` import found during fuzzing [CppInteropFuzzing]
14+
// CHECK:STDERR: import Cpp library "file.h";
15+
// CHECK:STDERR: ^~~~~~
16+
// CHECK:STDERR:
17+
import Cpp library "file.h";

toolchain/codegen/testdata/assembly/basic.carbon

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Exceptions. See /LICENSE for license information.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44
//
5-
// ARGS: compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=- %s
5+
// ARGS: --include-diagnostic-kind compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=- %s
66
//
77
// To test this file alone, run:
88
// bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/codegen/testdata/assembly/basic.carbon

toolchain/codegen/testdata/fail_target_triple.carbon

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Exceptions. See /LICENSE for license information.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44
//
5-
// ARGS: compile --no-prelude-import --target=x86_687-unknown-linux-gnu --output=- %s
5+
// ARGS: --include-diagnostic-kind compile --no-prelude-import --target=x86_687-unknown-linux-gnu --output=- %s
66
// No autoupdate because the message comes from LLVM.
77
// To test this file alone, run:
88
// bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/codegen/testdata/fail_target_triple.carbon

toolchain/codegen/testdata/objcode/basic.carbon

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Exceptions. See /LICENSE for license information.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44
//
5-
// ARGS: compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=%t %s
5+
// ARGS: --include-diagnostic-kind compile --no-prelude-import --target=x86_64-unknown-linux-gnu --output=%t %s
66
//
77
// TODO: Add a way to write some basic tests for object file outputs.
88
// AUTOUPDATE

toolchain/diagnostics/BUILD

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,15 @@ cc_test(
8181
],
8282
)
8383

84+
cc_library(
85+
name = "file_diagnostics",
86+
hdrs = ["file_diagnostics.h"],
87+
deps = [
88+
":diagnostic_emitter",
89+
"@llvm-project//llvm:Support",
90+
],
91+
)
92+
8493
cc_library(
8594
name = "format_providers",
8695
srcs = ["format_providers.cpp"],

toolchain/diagnostics/coverage_test.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ constexpr DiagnosticKind UntestedDiagnosticKinds[] = {
2424
DiagnosticKind::TestDiagnostic,
2525
DiagnosticKind::TestDiagnosticNote,
2626

27-
// Driver specific.
28-
DiagnosticKind::CppInteropFuzzing,
27+
// Diagnosing erroneous install conditions, but test environments are
28+
// typically correct.
29+
DiagnosticKind::CompilePreludeManifestError,
30+
DiagnosticKind::DriverInstallInvalid,
2931

3032
// These diagnose filesystem issues that are hard to unit test.
3133
DiagnosticKind::ErrorReadingFile,

0 commit comments

Comments
 (0)