[PATCH] D25731: [analyzer] NumberObjectConversion: Support OSNumber and CFNumberRef.

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp:111
+  QualType ObjT = (IsCpp || IsObjC)
+                      ? Obj->getType().getCanonicalType().getUnqualifiedType()
+                      : Obj->getType();
----------------
NoQ wrote:
> zaks.anna wrote:
> > Why do we need a case split here? Would calling getCanonicalType().getUnqualifiedType() result in a wrong result for ObjC?
> It'd result in a wrong result for `CFNumberRef`, which is a typedef we shouldn't remove (turning this into `struct __CFNumber *` would be ugly).
> 
> I'd probably rather avoid the case split by removing the `.getCanonicalType()` part, because rarely anybody makes typedefs for `NSNumber*` or `OSBoolean*` etc (such as in tests with the word "sugared").
> 
> Or i could descend down to the necessary typedef for C objects, discarding all user's typedefs but not library typedefs. But that'd be even more complicated and probably not very useful for such minor matter.
> It'd result in a wrong result for CFNumberRef, which is a typedef we shouldn't 
> remove (turning this into struct __CFNumber * would be ugly).
I see. Displaying "CFNumberRef" is definitely the right thing to do! Please, add a comment to explain this.


https://reviews.llvm.org/D25731