Skip to content

Commit

Permalink
Use nullptr for nil in C++11.
Browse files Browse the repository at this point in the history
  • Loading branch information
davidchisnall committed Feb 23, 2018
1 parent 7539d7e commit 3c036b3
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion objc/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,11 @@ typedef struct
#ifdef __GNUC
# define _OBJC_NULL_PTR __null
#elif defined(__cplusplus)
# define _OBJC_NULL_PTR 0
# if __has_feature(cxx_nullptr)
# define _OBJC_NULL_PTR nullptr
# else
# define _OBJC_NULL_PTR 0
# endif
#else
# define _OBJC_NULL_PTR ((void*)0)
#endif
Expand Down

5 comments on commit 3c036b3

@DHowett-MSFT
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT commented on 3c036b3 Feb 24, 2018

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:

#ifdef nil
#undef nil // suppress compiler-provided definition, if there is one
#endif
#include "objc/runtime.h"
int main() {
        void (^hello)();
        hello == nil;
        return 0;
}
root@88add234ecf8:/libobjc2# clang++ -std=c++11 -c -o test.o -x objective-c++ test.cpp -fblocks
test.cpp:4:8: error: invalid operands to binary expression ('void (^)()' and 'id')
        hello == nil;
        ~~~~~ ^  ~~~
1 error generated.

We fixed it by removing the cast, which didn't seem to impact plain C or plain Objective-C consumers.

@davidchisnall
Copy link
Member Author

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 of NULL as a symptom of confusing ObjC pointers and bare pointers (e.g. char* and NSString*), but maybe compiler warnings are good enough to catch those now.

@DHowett-MSFT
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT commented on 3c036b3 Feb 24, 2018

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 assignment hello = nil as you'd expect it to if the types were truly immiscible.

@DHowett-MSFT
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT commented on 3c036b3 Feb 24, 2018

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 like nullptrTy == blockTy and blockTy == ptrVoidTy. It can easily be augmented to support objcObjectTy == blockTy.

I'll roll this up into our Clang changes.

@davidchisnall
Copy link
Member Author

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.

Please sign in to comment.