-
Notifications
You must be signed in to change notification settings - Fork 119
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
7539d7e
commit 3c036b3
Showing
1 changed file
with
5 additions
and
1 deletion.
There are no files selected for viewing
This file contains 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
3c036b3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had issues comparing block types against nil as defined on line 211 (new) here; since it's of type
id
it was failing the stricter requirements on comparison types:We fixed it by removing the cast, which didn't seem to impact plain C or plain Objective-C consumers.
3c036b3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apple seems to have done the same thing. I don't really like it, because it means that we lose some type checking - I've found real bugs with people misusing
nil
instead ofNULL
as a symptom of confusing ObjC pointers and bare pointers (e.g.char*
andNSString*
), but maybe compiler warnings are good enough to catch those now.3c036b3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is better fixed in the compiler: block types should be more readily coercible to
id
during comparison when compiling as Objective-C/++. Adding to the above example, clang doesn't carp about the assignmenthello = nil
as you'd expect it to if the types were truly immiscible.3c036b3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SemaExpr::CheckCompareOperands
(SemaExpr.cpp:9724) handles cases likenullptrTy == blockTy
andblockTy == ptrVoidTy
. It can easily be augmented to supportobjcObjectTy == blockTy
.I'll roll this up into our Clang changes.
3c036b3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Add me as a reviewer and I'm happy to commit that if no one from Apple complains.