NoQ marked an inline comment as done.
NoQ added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp:149
BugReporter &BR) const {
MatchFinder F;
Callback CB(this, BR, AM.getAnalysisDeclContext(D));
----------------
alexshap wrote:
> probably it would make sense to move "MatchFinder F;" to the line 276 (closer
> to the place where it's actually being used)(or maybe i'm missing smth ?)
Yep, great idea.
================
Comment at: test/Analysis/number-object-conversion.c:14
+ if (p) {} // expected-warning{{Converting 'CFNumberRef' to a plain boolean
value for branching; please compare the pointer to NULL instead to suppress
this warning}}
+ if (!p) {} // expected-warning{{Converting 'CFNumberRef' to a plain boolean
value for branching; please compare the pointer to NULL instead to suppress
this warning}}
+ p ? 1 : 2; // expected-warning{{Converting 'CFNumberRef' to a plain boolean
value for branching; please compare the pointer to NULL instead to suppress
this warning}}
----------------
zaks.anna wrote:
> How about:
> "Converting 'CFNumberRef' pointer to a plain boolean value; instead, compare
> the pointer to NULL or compare to the encapsulated scalar value"
>
> - I've added "pointer".
> - I would remove "for branching". Does it add anything?
> - Also, we should remove "please" as it makes the warning text longer.
>
> I would remove "for branching". Does it add anything?
Because there's otherwise no obvious boolean value around, i wanted to point
out what exactly is going on.
> or compare to the encapsulated scalar value
They don't necessarily compare the value. Maybe "take"?
> I've added "pointer".
Not sure it's worth keeping for other objects ("`'NSNumber *' pointer`" sounds
like a pointer to a pointer).
https://reviews.llvm.org/D25731
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
