-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Emit compiler diagnostics #3867
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -612,7 +612,40 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate | |
// next we want to try and scoop out any errors from the output (if reasonable size, otherwise this will be very slow), | ||
// so they can later be passed to the advice provider in case of failure. | ||
if output.utf8.count < 1024 * 10 { | ||
let regex = try! RegEx(pattern: #".*(error:[^\n]*)\n.*"#, options: .dotMatchesLineSeparators) | ||
do { | ||
let regex = try! RegEx(pattern: #"^(.*):(\d+):(\d+):\s*(error|warning|note|remark)\S?\s*(.*)$"#, options: .anchorsMatchLines) | ||
for match in regex.matchGroups(in: output) { | ||
if let filePath = try? AbsolutePath(validating: match[0]) { | ||
|
||
var severity: Basics.Diagnostic.Severity { | ||
switch match[3] { | ||
case "error": | ||
return .error | ||
case "warning": | ||
return .warning | ||
case "note": | ||
return .info | ||
case "remark": | ||
return .info | ||
default: | ||
return .debug | ||
} | ||
} | ||
|
||
var metadata = ObservabilityMetadata() | ||
metadata.fileLocation = FileLocation(filePath, line: Int(match[1]), column: Int(match[2])) | ||
self.observabilityScope.emit( | ||
Basics.Diagnostic( | ||
severity: severity, | ||
message: match[4], | ||
metadata: metadata | ||
) | ||
) | ||
} | ||
} | ||
} | ||
|
||
let regex = try! RegEx(pattern: #"(error:[^\n]*)\n.*"#, options: .dotMatchesLineSeparators) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. safe. not my line though, I just moved it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, with a literal string to a RegEx, |
||
for match in regex.matchGroups(in: output) { | ||
self.errorMessagesByTarget[parser.targetName] = (self.errorMessagesByTarget[parser.targetName] ?? []) + [match[0]] | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -859,13 +859,24 @@ extension ObservabilityMetadata { | |
public struct FileLocation: Equatable, CustomStringConvertible { | ||
public let file: AbsolutePath | ||
public let line: Int? | ||
public let column: Int? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventually I think we will want line/column ranges here (as the .dia format supports) for that's for a later PR. Just a note about the direction I think we'll end up going here. |
||
|
||
public init(_ file: AbsolutePath, line: Int?) { | ||
public init(_ file: AbsolutePath, line: Int?, column: Int? = nil) { | ||
self.file = file | ||
self.line = line | ||
self.column = column | ||
} | ||
|
||
public var description: String { | ||
"\(self.file)\(self.line?.description.appending(" ") ?? "")" | ||
var desc = self.file.description | ||
if let line = line { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
desc += ":\(line)" | ||
} | ||
|
||
if let column = column { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
desc += ":\(column)" | ||
} | ||
|
||
return desc | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we guard for length of match? seems safer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's necessary in this case, since it comes from a RegEx created from a string literal. There will always be five matches unless there's a bug in
matchGroups(in:)
. Perhaps we can assert that, but it doesn't seem as if there is any runtime condition here that could cause this to fail.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully at some point Swift will have support for regular expressions where the matches can be returned as tuples so the type system knows how many there are.