SILGen: Ensure Foundation is publicly imported before referencing NSError in code gen
#79487
Conversation
|
@swift-ci Please smoke test |
|
@jckarter I would appreciate your review here. It would be possible to use the module-wide dependency information if it's preferable at the SIL level. The current use of source file local imports (and the re-exports) may limit this optimization more than necessary. |
|
Interesting. I'm surprised that we never have had a |
|
Me too, however most references to built-ins like this are for stdlib services, in that case the check is less useful in practice. Still I'm open to hear if there's something already in place for this in SILGen or an approach more appropriate to this component. |
Revisit the optimization that provides a fast path for instances of `NSError` when erasing the `Error` type in `emitExistentialErasure`. It generated references to `NSError` when the `Foundation` module was loaded, no matter how it was imported. This lead to deserialization failures at reading the swiftmodule when that reference was added to inlinable code while `Foundation` was not a public dependency. Fix this crash by limiting the optimization to all non-inlinable code and only inlinable code from a module with a public dependency on `Foundation`. This is the similar check we apply to user written inlinable code, however here we use the module-wide dependency instead of per file imports. rdar://142438679
xymus
force-pushed
the
limit-nserror-optimization-to-import
branch
from
d75993b to
db7126c
Compare
|
Updated the logic to enable the optimization if there's any public dependency on Foundation at the module level, instead of the file level. Also added a few more test cases. @swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
|
@swift-ci Please smoke test Linux |
|
Does the fallback path do the right thing for an opaque |
Ensure that a function manipulating an existential Error still supports NSError types with or without the fast-path optimization.
|
I added a runtime test to ensure we preserve the expected behavior for an NSError instance with or without the optimization modified here. @swift-ci Please smoke test |
Revisit the optimization that provides a fast path for instances of
NSErrorwhen erasing theErrortype inemitExistentialErasure. It generated references toNSErrorwhen theFoundationmodule was loaded, no matter how it was imported. This lead to deserialization failures at reading the swiftmodule when that reference was added to inlinable code whileFoundationwas not a public dependency.Fix this crash by limiting the optimization to all non-inlinable code and only inlinable code from a file with a public dependency on
Foundation. This is the similar check we apply to user written inlinable code.rdar://142438679