diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9c5c81ae9c34..445f5b70308e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -95,6 +95,16 @@ repos: entry: scripts/check_sha_filenames.py language: python files: ^.*/fuzzer_corpus/.*$ + - id: check-toolchain-diagnostics + name: Check toolchain diagnostics + entry: toolchain/diagnostics/check_diagnostics.py + language: python + files: | + (?x)^( + toolchain/.*\.cpp| + toolchain/.*\.h| + toolchain/diagnostics/diagnostic_registry\.def + )$ # Run linters last, as formatters and other checks may fix issues. - repo: local diff --git a/toolchain/diagnostics/check_diagnostics.py b/toolchain/diagnostics/check_diagnostics.py new file mode 100755 index 000000000000..b202c61e5241 --- /dev/null +++ b/toolchain/diagnostics/check_diagnostics.py @@ -0,0 +1,117 @@ +#!/usr/bin/env python3 + +"""Checks diagnostic use. + +Validates that each diagnostic declared with CARBON_DIAGNOSTIC_KIND is +referenced by one (and only one) CARBON_DIAGNOSTIC. +""" + +__copyright__ = """ +Part of the Carbon Language project, under the Apache License v2.0 with LLVM +Exceptions. See /LICENSE for license information. +SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +""" + +import collections +from concurrent import futures +import itertools +from pathlib import Path +import os +import re +import sys +from typing import Dict, List, NamedTuple, Set + + +# Example or test diagnostics, ignored because they're expected to not pass. +IGNORED = set(["MyDiagnostic", "TestDiagnostic"]) + + +class Location(NamedTuple): + """A location for a diagnostic.""" + + def __str__(self) -> str: + return f"{str(self.path)}:{self.line}" + + path: Path + line: int + + +def load_diagnostic_kind() -> Set[str]: + """Returns the set of declared diagnostic kinds. + + This isn't validated for uniqueness because the compiler does that. + """ + path = Path("toolchain/diagnostics/diagnostic_registry.def") + content = path.read_text() + decls = set(re.findall(r"CARBON_DIAGNOSTIC_KIND\((.+)\)", content)) + return decls.difference(IGNORED) + + +def load_diagnostic_uses_in( + path: Path, +) -> Dict[str, List[Location]]: + """Returns the path's CARBON_DIAGNOSTIC uses.""" + content = path.read_text() + + # Keep a line cursor so that we don't keep re-scanning the file. + line = 1 + line_offset = 0 + + found: Dict[str, List[Location]] = collections.defaultdict(lambda: []) + for m in re.finditer(r"CARBON_DIAGNOSTIC\(\s*(\w+),", content): + diag = m.group(1) + if diag in IGNORED: + continue + line += content.count("\n", line_offset, m.start()) + line_offset = m.start() + found[diag].append(Location(path, line)) + return found + + +def load_diagnostic_uses() -> Dict[str, List[Location]]: + """Returns all CARBON_DIAGNOSTIC uses.""" + globs = itertools.chain( + *[Path("toolchain").glob(f"**/*.{ext}") for ext in ("h", "cpp")] + ) + with futures.ThreadPoolExecutor() as exec: + results = exec.map(load_diagnostic_uses_in, globs) + + found: Dict[str, List[Location]] = collections.defaultdict(lambda: []) + for result in results: + for diag, locations in result.items(): + found[diag].extend(locations) + return found + + +def check_uniqueness(uses: Dict[str, List[Location]]) -> bool: + """If any diagnostic is non-unique, prints an error and returns true.""" + has_errors = False + for diag in sorted(uses.keys()): + if len(uses[diag]) > 1: + print(f"Non-unique diagnostic {diag}:", file=sys.stderr) + for loc in uses[diag]: + print(f" - {loc}", file=sys.stderr) + has_errors = True + return has_errors + + +def check_unused(decls: Set[str], uses: Dict[str, List[Location]]) -> bool: + """If any diagnostic is unused, prints an error and returns true.""" + unused = decls.difference(uses.keys()) + for diag in sorted(unused): + print(f"Unused diagnostic: {diag}") + return False + + +def main() -> None: + # Run from the repo root. + os.chdir(Path(__file__).parent.parent.parent) + decls = load_diagnostic_kind() + uses = load_diagnostic_uses() + + if any([check_uniqueness(uses), check_unused(decls, uses)]): + exit(1) + + +if __name__ == "__main__": + main() diff --git a/toolchain/diagnostics/diagnostic_registry.def b/toolchain/diagnostics/diagnostic_registry.def index 75fca2a5947d..f312395b1b51 100644 --- a/toolchain/diagnostics/diagnostic_registry.def +++ b/toolchain/diagnostics/diagnostic_registry.def @@ -61,11 +61,9 @@ CARBON_DIAGNOSTIC_KIND(ExpectedStructLiteralField) CARBON_DIAGNOSTIC_KIND(ExpectedVariableDeclaration) CARBON_DIAGNOSTIC_KIND(ExpectedVariableName) CARBON_DIAGNOSTIC_KIND(OperatorRequiresParentheses) -CARBON_DIAGNOSTIC_KIND(StackLimitExceeded) CARBON_DIAGNOSTIC_KIND(UnaryOperatorHasWhitespace) CARBON_DIAGNOSTIC_KIND(UnaryOperatorRequiresWhitespace) CARBON_DIAGNOSTIC_KIND(UnexpectedTokenAfterListElement) -CARBON_DIAGNOSTIC_KIND(UnexpectedTokenInCodeBlock) CARBON_DIAGNOSTIC_KIND(UnrecognizedDeclaration) // Package-related diagnostics. @@ -77,6 +75,7 @@ CARBON_DIAGNOSTIC_KIND(ExpectedSemiToEndPackageDirective) // For-specific diagnostics. CARBON_DIAGNOSTIC_KIND(ExpectedIn) +CARBON_DIAGNOSTIC_KIND(ExpectedInNotColon) // Interface-specific diagnostics. CARBON_DIAGNOSTIC_KIND(ExpectedInterfaceName) diff --git a/toolchain/parser/parser.cpp b/toolchain/parser/parser.cpp index 2f4fc62f57e5..65ae76458c92 100644 --- a/toolchain/parser/parser.cpp +++ b/toolchain/parser/parser.cpp @@ -28,6 +28,9 @@ CARBON_DIAGNOSTIC(ExpectedParenAfter, Error, "Expected `(` after `{0}`.", CARBON_DIAGNOSTIC(ExpectedSemiAfterExpression, Error, "Expected `;` after expression."); +CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error, + "Unrecognized declaration introducer."); + // A relative location for characters in errors. enum class RelativeLocation : int8_t { Around, @@ -738,8 +741,6 @@ auto Parser::HandleDeclarationLoopState() -> void { break; } default: { - CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error, - "Unrecognized declaration introducer."); emitter_->Emit(*position_, UnrecognizedDeclaration); auto cursor = *position_; auto semi = SkipPastLikelyEnd(cursor); @@ -1236,8 +1237,6 @@ auto Parser::HandleInterfaceDefinitionLoopState() -> void { break; } default: { - CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error, - "Unrecognized declaration introducer."); emitter_->Emit(*position_, UnrecognizedDeclaration); if (auto semi = SkipPastLikelyEnd(*position_)) { AddLeafNode(ParseNodeKind::EmptyDeclaration(), *semi, @@ -1851,8 +1850,9 @@ auto Parser::HandleVarFinishAsForState() -> void { if (PositionIs(TokenKind::In())) { end_token = Consume(); } else if (PositionIs(TokenKind::Colon())) { - CARBON_DIAGNOSTIC(ExpectedIn, Error, "`:` should be replaced by `in`."); - emitter_->Emit(*position_, ExpectedIn); + CARBON_DIAGNOSTIC(ExpectedInNotColon, Error, + "`:` should be replaced by `in`."); + emitter_->Emit(*position_, ExpectedInNotColon); state.has_error = true; end_token = Consume(); } else {