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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
