MTC added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector<CallDescription, 4> CStrFnFamily = {
+ {"std::basic_string<char>::c_str"}, {"std::basic_string<char32_t>::c_str"},
----------------
NoQ wrote:
> xazax.hun wrote:
> > I am not sure if this is the right solution in case of this check. We
> > should track `c_str` calls regardless of what the template parameter is, so
> > supporting any instantiation of `basic_string` is desired. This might not
> > be the case, however, for other checks.
> >
> > If we think it is more likely we do not care about the template arguments,
> > maybe a separate API could be used, where we pass the qualified name of the
> > class separately without the template arguments.
> > Alternatively, we could use matches name so the users could use regexps.
> >
> > At this point I also wonder what isCalled API gives us compared to
> > matchers? Maybe it is more convenient to use than calling a `match`. Also,
> > isCalled API has an IdentifierInfo cached which could be used for
> > relatively efficient checks.
> >
> > @NoQ what do you think?
> >
> >
> I agree that it's better to match the chain of classes and namespaces (in as
> much detail as the user cares to provide) and discard template parameters.
>
> For example, i wish that a `CallDescription` that's defined as `{"std",
> "basic_string", "c_str"}` would be able to match both `std::string::c_str()`
> and `std::__1::string::c_str()`.
Yea, checker writers may can't provide all the template arguments, and it's not
convenient to use!
I will try to give a better solution!
================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
auto *TypeDecl = TypedR->getValueType()->getAsCXXRecordDecl();
if (TypeDecl->getName() != "basic_string")
return;
----------------
xazax.hun wrote:
> If we check for fully qualified names, this check might be redundant.
Right, thanks!
================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273
+ auto Matches =
+ match(namedDecl(hasName(CD.FuncName)).bind("match_qualified_name"), *ND,
+ LCtx->getAnalysisDeclContext()->getASTContext());
----------------
xazax.hun wrote:
> Doesn't match also traverse the subtree of the AST? We only need to check if
> that particular node matches the qualified name. I wonder if we could, during
> the traversal, find another node that is a match unintentionally.
Yea, this maybe a problem, I will test whether this is going to happen and give
a fine-grained match. Thanks!
Repository:
rC Clang
https://reviews.llvm.org/D48027
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
