[SPI] Requestify, inherit SPI groups and better diagnostics #30215
Conversation
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
| } | ||
|
|
||
| llvm::Expected<llvm::ArrayRef<Identifier>> | ||
| SPIGroupsRequest::evaluate(Evaluator &evaluator, const Decl *decl) const { |
There was a problem hiding this comment.
There could be a cycle in this request. Could we diagnose that explicitly?
There was a problem hiding this comment.
I'll have to look into this... Do you see a direct cycle that could be created here?
There was a problem hiding this comment.
Not currently. My concern is when we add more recursive calls of getSPIGroups() in this function, we may end up with unexpected cycles. The request template should allow us to diagnose cycle by overriding void diagnoseCycle(DiagnosticEngine &diags)
| @@ -127,9 +127,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, | |||
| if (D->getDeclContext()->isLocalContext()) | |||
| return false; | |||
|
|
|||
| // Public declarations are OK. | |||
| // Public non-SPI declarations are OK. | |||
There was a problem hiding this comment.
Do we diagnose @SPI decl inside a non-resilient type marked of @frozen? it should be disallowed.
There was a problem hiding this comment.
This should be diagnosed. There's a test in local_spi_decls.swift but it could be extended to cover more edge cases.
There was a problem hiding this comment.
Sorry, I should be more specific here. Say when we have a frozen struct with three fields and the second field is marked as SPI. This means in the API world, the third field will be referenced as the second field in runtime even the user thinks they are referencing the third field by name. This could lead to unintended runtime behavior. I wonder whether this pattern is diagnosed in local_spi_decls.swift.
Improve SPI support on four main points:
@_spiinstead ofinternal.@_spi.