Use fatalError instead of precondition for missing permissions on .create#307
Open
adityasingh2400 wants to merge 1 commit into
Open
Use fatalError instead of precondition for missing permissions on .create#307adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
FileDescriptor.open traps when options contains .create but permissions is nil. The previous precondition could elide its message in optimized builds, leaving users to hunt through source to diagnose the crash. Use fatalError with a descriptive message so the reason is always present in the crash dump, and document the trapping behavior. Fixes apple#78
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FileDescriptor.open(_:_:options:permissions:retryOnInterrupt:)traps whenoptionscontains.createbutpermissionsisnil. As reported in #78,this trap was raised with
precondition, whose failure message can be elidedin optimized builds. When that happens the program aborts without any
indication of why, forcing users to dig through the source to discover that
.createrequires a non-nilpermissionsvalue.The reporter concluded this is a programmer error (so
throwsis notappropriate), but that the crash should still carry a clear message in the
crash dump.
Change
precondition(!options.contains(.create), ...)check in theinternal
_openimplementation with an explicitif options.contains(.create) { fatalError(...) }. Unlikeprecondition,fatalErroralways fires and always prints its message regardless ofoptimization level, so the diagnostic is guaranteed to appear in the crash
dump.
"FileDescriptor.open: 'permissions' must not be nil when 'options' contains '.create'".permissions:parameter docs of thethree public
openoverloads, so the requirement is discoverable from theAPI reference.
The change is confined to the existing trapping code path; no public API,
signatures, or non-trapping behavior change.
Verification
swift buildsucceeds (forced recompile ofFileOperations.swift).swift test --filter FileOperationsTestpasses: 8/8 tests.TemporaryPathTest.testNotInSlashTmp, which is anenvironmental failure unrelated to this change (it asserts the temp dir is
not under
/tmp, and fails identically on an unmodified checkout when thetest process runs from
/tmp).Fixes #78