Prazek added inline comments.
================
Comment at: clang-tidy/cert/LimitedRandomnessCheck.cpp:22-23
@@ +21,4 @@
+ Finder->addMatcher(
+ declRefExpr(hasDeclaration(functionDecl(namedDecl(hasName("::rand")),
+ parameterCountIs(0))))
+ .bind("randomGenerator"),
----------------
aaron.ballman wrote:
> Prazek wrote:
> > aaron.ballman wrote:
> > > This should be looking at a callExpr() rather than a declRefExpr(),
> > > should it not?
> > I was also thinking about this, but this is actually better, because it
> > will also match with binding rand with function pointer.
> True, but a DeclRefExpr doesn't mean it's a function call. Binding the
> function is not contrary to the CERT rule, just calling it. For instance, the
> following pathological case will be caught by this check:
> ```
> if (std::rand) {}
> ```
> The behavior of this check should be consistent with cert-env33-c, which only
> looks at calls. (If we really care about bound functions, we'd need flow
> control analysis, and I think that's overkill for both of those checks, but
> wouldn't be opposed to someone writing the flow analysis if they really
> wanted to.)
It would indeed fire on this pathological case, but I don't think we should
care about cases like this, because no one is writing code like this (and if he
would then it would probably be a bug).
I don't think that there is much code that binds pointer to std::rand either,
but I think it would be good to display warning for this, because even if the
function would be never called, then it means that this is a bug, and if it
would be called then it would be nice to tell user that rand might be used here.
Anyway I don't oppose for changing it to callExpr, but I think it is better
this way.
Repository:
rL LLVM
https://reviews.llvm.org/D22346
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
