Skip to content

[ASTGen] Simplify diagnostics framework #77404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions lib/ASTGen/Sources/ASTGen/ASTGen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Boxed<Value> {
}

struct ASTGenVisitor {
fileprivate let diagnosticEngine: BridgedDiagnosticEngine
let diagnosticEngine: BridgedDiagnosticEngine

let base: UnsafeBufferPointer<UInt8>

Expand Down Expand Up @@ -273,24 +273,6 @@ extension ASTGenVisitor {
}
}

extension ASTGenVisitor {
/// Emits the given diagnostic via the C++ diagnostic engine.
@inline(__always)
func diagnose(_ diagnostic: Diagnostic) {
emitDiagnostic(
diagnosticEngine: self.diagnosticEngine,
sourceFileBuffer: self.base,
diagnostic: diagnostic,
diagnosticSeverity: diagnostic.diagMessage.severity
)
}

/// Emits the given diagnostics via the C++ diagnostic engine.
func diagnoseAll(_ diagnostics: [Diagnostic]) {
diagnostics.forEach(diagnose)
}
}

// Forwarding overloads that take optional syntax nodes. These are defined on demand to achieve a consistent
// 'self.generate(foo: FooSyntax)' recursion pattern between optional and non-optional inputs.
extension ASTGenVisitor {
Expand Down
20 changes: 8 additions & 12 deletions lib/ASTGen/Sources/ASTGen/Decls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ extension ASTGenVisitor {
case .`init`:
return .`init`
default:
self.diagnose(Diagnostic(node: specifier, message: UnknownAccessorSpecifierError(specifier)))
self.diagnose(.unknownAccessorSpecifier(specifier))
return nil
}
}
Expand Down Expand Up @@ -448,7 +448,7 @@ extension ASTGenVisitor {
let storage = primaryVar.asAbstractStorageDecl
storage.setAccessors(generate(accessorBlock: accessors, for: storage))
} else {
self.diagnose(Diagnostic(node: binding.pattern, message: NonTrivialPatternForAccessorError()))
self.diagnose(.nonTrivialPatternForAccessor(binding.pattern))
}
}
return BridgedPatternBindingEntry(
Expand Down Expand Up @@ -667,9 +667,7 @@ extension ASTGenVisitor {
fixity = value
} else {
fixity = .infix
self.diagnose(
Diagnostic(node: node.fixitySpecifier, message: UnexpectedTokenKindError(token: node.fixitySpecifier))
)
self.diagnose(.unexpectedTokenKind(token: node.fixitySpecifier))
}

return .createParsed(
Expand Down Expand Up @@ -712,9 +710,7 @@ extension ASTGenVisitor {
}

func diagnoseDuplicateSyntax(_ duplicate: some SyntaxProtocol, original: some SyntaxProtocol) {
self.diagnose(
Diagnostic(node: duplicate, message: DuplicateSyntaxError(duplicate: duplicate, original: original))
)
self.diagnose(.duplicateSyntax(duplicate: duplicate, original: original))
}

let body = node.groupAttributes.reduce(into: PrecedenceGroupBody()) { body, element in
Expand All @@ -735,7 +731,7 @@ extension ASTGenVisitor {
body.lowerThanRelation = relation
}
default:
return self.diagnose(Diagnostic(node: keyword, message: UnexpectedTokenKindError(token: keyword)))
return self.diagnose(.unexpectedTokenKind(token: keyword))
}
case .precedenceGroupAssignment(let assignment):
if let current = body.assignment {
Expand All @@ -757,7 +753,7 @@ extension ASTGenVisitor {
if let value = BridgedAssociativity(from: token.keywordKind) {
associativityValue = value
} else {
self.diagnose(Diagnostic(node: token, message: UnexpectedTokenKindError(token: token)))
self.diagnose(.unexpectedTokenKind(token: token))
associativityValue = .none
}
} else {
Expand All @@ -769,7 +765,7 @@ extension ASTGenVisitor {
if token.keywordKind == .true {
assignmentValue = true
} else {
self.diagnose(Diagnostic(node: token, message: UnexpectedTokenKindError(token: token)))
self.diagnose(.unexpectedTokenKind(token: token))
assignmentValue = false
}
} else {
Expand Down Expand Up @@ -824,7 +820,7 @@ extension ASTGenVisitor {
if let value = BridgedImportKind(from: specifier.keywordKind) {
importKind = value
} else {
self.diagnose(Diagnostic(node: specifier, message: UnexpectedTokenKindError(token: specifier)))
self.diagnose(.unexpectedTokenKind(token: specifier))
importKind = .module
}
} else {
Expand Down
161 changes: 86 additions & 75 deletions lib/ASTGen/Sources/ASTGen/Diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,65 +13,90 @@
import SwiftDiagnostics
import SwiftSyntax

protocol ASTGenError: DiagnosticMessage {}
extension ASTGenVisitor {
/// Emits the given ASTGen diagnostic via the C++ diagnostic engine.
func diagnose(_ message: ASTGenDiagnostic, highlights: [Syntax]? = nil, notes: [Note] = [], fixIts: [FixIt] = []) {
self.diagnose(Diagnostic(
node: message.node,
message: message,
highlights: highlights,
notes: notes,
fixIts: fixIts
))
}

extension ASTGenError {
var diagnosticID: MessageID {
MessageID(domain: "ASTGen", id: "\(Self.self)")
/// Emits the given diagnostic via the C++ diagnostic engine.
func diagnose(_ diagnostic: Diagnostic) {
emitDiagnostic(
diagnosticEngine: self.diagnosticEngine,
sourceFileBuffer: self.base,
diagnostic: diagnostic,
diagnosticSeverity: diagnostic.diagMessage.severity
)
}

var severity: DiagnosticSeverity { .error }
/// Emits the given diagnostics via the C++ diagnostic engine.
func diagnoseAll(_ diagnostics: [Diagnostic]) {
diagnostics.forEach(diagnose)
}
}

/// An error emitted when a token is of an unexpected kind.
struct UnexpectedTokenKindError: ASTGenError {
let token: TokenSyntax
private let parent: Syntax
struct ASTGenDiagnostic: DiagnosticMessage {
var node: Syntax
var message: String
var severity: DiagnosticSeverity
var messageID: String

init(token: TokenSyntax) {
guard let parent = token.parent else {
preconditionFailure("Expected a child (not a root) token")
}
var diagnosticID: MessageID {
MessageID(domain: "ASTGen", id: messageID)
}

self.token = token
self.parent = parent
init(node: some SyntaxProtocol, message: String, severity: DiagnosticSeverity = .error, messageID: String) {
self.node = Syntax(node)
self.message = message
self.severity = severity
self.messageID = messageID
}

var message: String {
return """
unexpected token kind for token:
\(self.token.debugDescription)
in parent:
\(self.parent.debugDescription(indentString: " "))
"""
fileprivate init(node: some SyntaxProtocol, message: String, severity: DiagnosticSeverity = .error, function: String = #function) {
// Extract messageID from the function name.
let messageID = String(function.prefix(while: { $0 != "(" }))
self.init(node: node, message: message, severity: severity, messageID: messageID)
}
}

/// An error emitted when an optional child token is unexpectedly nil.
struct MissingChildTokenError: ASTGenError {
let parent: Syntax
let kindOfTokenMissing: TokenKind
extension ASTGenDiagnostic {
/// An error emitted when a token is of an unexpected kind.
static func unexpectedTokenKind(token: TokenSyntax) -> Self {
guard let parent = token.parent else {
preconditionFailure("Expected a child (not a root) token")
}

init(parent: some SyntaxProtocol, kindOfTokenMissing: TokenKind) {
self.parent = Syntax(parent)
self.kindOfTokenMissing = kindOfTokenMissing
return Self(
node: token,
message: """
unexpected token kind for token:
\(token.debugDescription)
in parent:
\(parent.debugDescription(indentString: " "))
"""
)
}

var message: String {
"""
missing child token of kind '\(self.kindOfTokenMissing)' in:
\(parent.debugDescription(indentString: " "))
"""
/// An error emitted when an optional child token is unexpectedly nil.
static func missingChildToken(parent: some SyntaxProtocol, kindOfTokenMissing: TokenKind) -> Self {
Self(
node: parent,
message: """
missing child token of kind '\(kindOfTokenMissing)' in:
\(parent.debugDescription(indentString: " "))
Comment on lines +91 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these messages intended to be multi-line. I don’t think we have multi-line diagnostics anywhere else at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to revisit here before enabling ASTGen by default.

"""
)
}
}

/// An error emitted when a syntax collection entry is encountered that is considered a duplicate of a previous entry
/// per the language grammar.
struct DuplicateSyntaxError: ASTGenError {
let duplicate: Syntax
let original: Syntax

init(duplicate: some SyntaxProtocol, original: some SyntaxProtocol) {
/// An error emitted when a syntax collection entry is encountered that is
/// considered a duplicate of a previous entry per the language grammar.
static func duplicateSyntax(duplicate: some SyntaxProtocol, original: some SyntaxProtocol) -> Self {
precondition(duplicate.kind == original.kind, "Expected duplicate and original to be of same kind")

guard let duplicateParent = duplicate.parent, let originalParent = original.parent,
Expand All @@ -80,42 +105,28 @@ struct DuplicateSyntaxError: ASTGenError {
preconditionFailure("Expected a shared syntax collection parent")
}

self.duplicate = Syntax(duplicate)
self.original = Syntax(original)
}

var message: String {
"""
unexpected duplicate syntax in list:
\(duplicate.debugDescription(indentString: " "))
previous syntax:
\(original.debugDescription(indentString: " "))
"""
}
}

struct NonTrivialPatternForAccessorError: ASTGenError {
var message: String {
"getter/setter can only be defined for a single variable"
}
}

struct UnknownAccessorSpecifierError: ASTGenError {
var specifier: TokenSyntax
init(_ specifier: TokenSyntax) {
self.specifier = specifier
return Self(
node: duplicate,
message: """
unexpected duplicate syntax in list:
\(duplicate.debugDescription(indentString: " "))
previous syntax:
\(original.debugDescription(indentString: " "))
"""
)
}

var message: String {
"unknown accessor specifier '\(specifier.text)'"
static func nonTrivialPatternForAccessor(_ pattern: some SyntaxProtocol) -> Self {
Self(
node: pattern,
message: "getter/setter can only be defined for a single variable"
)
}
}

struct RegexParserError: ASTGenError {
var message: String
init(_ message: String) {
self.message = message
static func unknownAccessorSpecifier(_ specifier: TokenSyntax) -> Self {
Self(
node: specifier,
message: "unknown accessor specifier '\(specifier.text)'"
)
}

var severity: DiagnosticSeverity { .error }
}
8 changes: 2 additions & 6 deletions lib/ASTGen/Sources/ASTGen/Exprs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -406,14 +406,10 @@ extension ASTGenVisitor {
func generate(functionCallExpr node: FunctionCallExprSyntax, postfixIfConfigBaseExpr: BridgedExpr? = nil) -> BridgedCallExpr {
if !node.arguments.isEmpty || node.trailingClosure == nil {
if node.leftParen == nil {
self.diagnose(
Diagnostic(node: node, message: MissingChildTokenError(parent: node, kindOfTokenMissing: .leftParen))
)
self.diagnose(.missingChildToken(parent: node, kindOfTokenMissing: .leftParen))
}
if node.rightParen == nil {
self.diagnose(
Diagnostic(node: node, message: MissingChildTokenError(parent: node, kindOfTokenMissing: .rightParen))
)
self.diagnose(.missingChildToken(parent: node, kindOfTokenMissing: .rightParen))
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/ASTGen/Sources/ASTGen/Types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ extension ASTGenVisitor {
specifierLoc: self.generateSourceLoc(specifier)
).asTypeRepr
} else {
self.diagnose(Diagnostic(node: specifier, message: UnexpectedTokenKindError(token: specifier)))
self.diagnose(.unexpectedTokenKind(token: specifier))
}
}

Expand Down