When creation of a syntax node using string interpolation failed, log the diagnostics #1790
Conversation
|
@swift-ci Please test |
ahoppen
force-pushed
the
ahoppen/log-string-interpolation-parsing-errors
branch
from
8267d1d to
42f9f5f
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
ahoppen
force-pushed
the
ahoppen/log-string-interpolation-parsing-errors
branch
from
42f9f5f to
cf6f146
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
1 similar comment
|
@swift-ci Please test Windows |
| Parsing a `\(Self.self)` node from string interpolation produced the following parsing errors. | ||
| Set a brakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure. |
There was a problem hiding this comment.
| Parsing a `\(Self.self)` node from string interpolation produced the following parsing errors. | |
| Set a brakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure. | |
| Parsing a `\(Self.self)` node from string interpolation produced the following parsing errors. | |
| Set a breakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure. |
| @@ -69,9 +69,27 @@ final class LinkageTest: XCTestCase { | |||
| .library("-lswiftCompatibilityConcurrency"), | |||
| .library("-lswiftCompatibilityPacks", condition: .mayBeAbsent("Only in newer compilers")), | |||
| .library("-lswiftCore"), | |||
| .library("-lswiftCoreFoundation", condition: .when(compilationCondition: .osLogDependency)), | |||
There was a problem hiding this comment.
We could just only run this check when we don't have the oslog dependency. We don't really care what's linked if we do.
There was a problem hiding this comment.
You mean put the entire test behind #if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY?
This test is starting to get a little odd because in practice we only ever run this test using swift test on macOS where we don’t set SWIFTSYNTAX_NO_OSLOG_DEPENDENCY, so it does raise a question about its value.
ahoppen
force-pushed
the
ahoppen/log-string-interpolation-parsing-errors
branch
from
cf6f146 to
e0b298b
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
It's not clear for me. How this OS log indicates the location? I think the information in the OS is the same as the parser diagnostics as #1758 , no? (edit) |
|
I feel it's Instead, I think |
… the diagnostics From the first feedback of developers starting to build macros, the number one mistake was to build incorrect syntax nodes, e.g. build an `ExprSyntax` for a declaration. Up until recently, these errors weren’t diagnosed at all. By now we at least issue an XCTest assertion if the expanded macro code contains syntax errors. But I think the issues are a lot easier to debug if we can emit an error at the location where the incorrect syntax node is being created. Since we can’t throw an error from string interpolation, use OSLog to log the parsing error to Xcode or OS console. When OSLog is not available or we are building using CMake (and thus don’t want to introduce any dependency on the SDK), silently accept the error as we do right now. Logging to stderr might be too fragile in case some application is expecting to populate stderr itself.
ahoppen
force-pushed
the
ahoppen/log-string-interpolation-parsing-errors
branch
from
e0b298b to
96557f7
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
|
@swift-ci Please test macOS |
From the first feedback of developers starting to build macros, the number one mistake was to build incorrect syntax nodes, e.g. build an
ExprSyntaxfor a declaration. For exampleUp until recently, these errors weren’t diagnosed at all. By now we at least issue an XCTest assertion if the expanded macro code contains syntax errors. But I think the issues are a lot easier to debug if we can emit an error at the location where the incorrect syntax node is being created.
Since we can’t throw an error from string interpolation, use OSLog to log the parsing error to Xcode or OS console. When OSLog is not available or we are building using CMake (and thus don’t want to introduce any dependency on the SDK), silently accept the error as we do right now. Logging to stderr might be too fragile in case some application is expecting to populate stderr itself.