Fix SPM integration. #746
Fix SPM integration. #746
Conversation
| block(subclass as! T.Type) // swiftlint:disable:this force_cast | ||
| } | ||
|
|
||
| guard let superclass = class_getSuperclass(subclass), |
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
Generated by 🚫 danger |
There was a problem hiding this comment.
Thanks so much, @sunshinejr! I think I'm not following some of your changes, though. Are you sure this is fixing the cause of the error? Or is it preventing the error case from being reached? See my questions below for details, and apologies in advance if I'm misunderstanding.
| } | ||
|
|
||
| guard let superclass = class_getSuperclass(subclass), | ||
| superclass === klass else { return } |
There was a problem hiding this comment.
The return here would halt enumeration of the classes defined in this bundle, no? Should this be a continue instead? Let me know if I'm misreading this 😅
There was a problem hiding this comment.
Yeah, you are right, sorry about that. I've just made it a simple if like it was before.
| guard let superclass = class_getSuperclass(subclass), | ||
| superclass === klass else { return } | ||
|
|
||
| block(subclass as! T.Type) // swiftlint:disable:this force_cast |
There was a problem hiding this comment.
I'm not sure I understand how this fixes the problem.
So, here's the code before:
// 1. Get the superclass.
superclass = class_getSuperclass(subclass)
// 2. If the superclass is the same as the class passed into this function...
if superclass === klass {
// 3. ...then force-cast the subclass.
block(subclass as! T.Type)
}And here's the new version:
// 1. Get the superclass and check if it's the same as the class passed into
// this function. If it isn't, exit early.
guard let superclass = class_getSuperclass(subclass),
superclass === klass else { return }
// 2. Otherwise, the superclass is the same as the class passed into this function...
// 3. ...then force-cast the subclass.
block(subclass as! T.Type)Reading the two, it seems like they basically do the same thing...?
However, I think maybe the difference is that you return instead of continue. So I think the actual assertion failure happens later in the iteration. In other words, maybe your change prevents the error because it prevents this loop from going through all of the classes, and therefore it never reaches the class that causes the error.
Or am I misreading this? Let me know, thanks!
|
@modocache the most important line didn't get the commit it seems, not sure why SourceTree didn't catch it, but yeah, now it is up. Sorry about that! So the line: var subclass, superclass: AnyClass!was the issue as it force-casted the superclass, even though it could be nil in some cases. I created a local check which in case of a nil will just skip it, instead of assigning to force-casted type and then crashing. So basically: var superclass: AnyClass!
superclass = class_getSuperclass(subclass) // which causes assigning nil to force-casted type when there is no superclass
if superclass === klass { // now we are accessing nil object in AnyClass! which results in bad access
block(subclass as! T.Type) // swiftlint:disable:this force_cast
}vs // in case of a nil, we just skip it
if let superclass = class_getSuperclass(subclass), superclass === klass {
block(subclass as! T.Type) // swiftlint:disable:this force_cast
}Let me know if it's clear now 😅 |
|
Excellent, thank you @sunshinejr! ❤️ |
Hey guys! First of all thank you so much for your constant work on Quick/Nimble, really appreciate that. Recently I was working extensively with SPM and wanted to try both Quick/Nimble with SPM. I encountered a runtime crash (bad access) on this line.
My environment:
Seems like the forcecast on the
superclasstype was creating a crash whenever there was no superclass. I've changed it to be safer so whenever there is no superclass, it will just skip it. If the code style is different from what you have right now, feel free to correct me/change it.Cheers!