Tighten NSKeyedArchiver retained NSError annotations#607
Conversation
Add narrow nullability annotations for the first Phase 4B core Foundation tranche without touching collection headers, module maps, or non-core surfaces. Keep the remaining judgment-call annotations conservative: - NSString legacy C-string decoding entry points stay nullable where decoding can return nil even though NULL C string inputs still raise. - NSURL initWithScheme:host:path: stays nullable because it funnels through initWithString:relativeToURL:, which can still fail for constructed non-ASCII URL text. - NSData bytes and mutableBytes stay nullable because zero-length concrete storage may legitimately expose a NULL backing pointer.
danjboyd
force-pushed
the
phase-15j-nskeyedarchiver-nserror-annotations
branch
from
79f75dc to
09bfdd7
Compare
rfm
left a comment
rfm
left a comment
There was a problem hiding this comment.
While he idea here looks basically good, we do have some problems:
We need a ChangeLog entry listing the changed files and summarising what has been done and why.
For the cases where the source code (rather than just headers) is changed to implement new functionality or to fix a bug, we really want reference documentation (either modifying existing doc or adding new stuff where, unfortunately, docs are currently missing). The gnustep reference documentation is automatically generated from specially formatted comments, so that's not hard.
I think the biggest issue is that the modified code doesn't compile on many platforms. From a quick look I think it's probably all gcc based systems. Currently NSObjCRuntime.h contains compatibility preprocessor definitions to cope with nullability compiler differences, so any file which hasn't imported that header will fail when it encounters an unknown keyword.
The quick/simple change would be to make sure the import takes place in any file where it's needed to build.
However, I think the current state of the code is a bit of a mess and ideally we'd improve it: putting compiler specific defines in NSObjcRuntime.h seems inappropriate, and we already have that kind of thing in GSVersionMacros.h (and possibly other places too). The file GSConfig.h (generated from GSConfig.h.in) is used to encapsulate most system-dependent variations which were discovered at configure time, so it might be that all the macros/defines to handler compiler specific dependencies should be moved to that file (which is imported pretty much everywhere).
Either solution would do though.
3 participants
Summary
Validation