MTC added a comment.
In https://reviews.llvm.org/D48027#1209844, @NoQ wrote:
> So i believe that one of the important remaining problems with
> `CallDescription` is to teach it to discriminate between global functions and
> methods. We can do it in a number of ways:
>
> 1. Make a special
NoQ added a comment.
In https://reviews.llvm.org/D48027#1207721, @rnkovacs wrote:
> I guess it is highly unlikely for someone to write namespaces like that, and
> if they do, I think they deserve to have them treated as a `std::string`.
Yup. On the other hand, if we specify our method as
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340407: [analyzer] Improve `CallDescription` to handle c++
method. (authored by henrywong, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48027?vs=161217=161938#toc
Repository:
MTC added a comment.
In https://reviews.llvm.org/D48027#1207645, @xazax.hun wrote:
> Sorry for the delay, I think this is OK to commit.
> As a possible improvement, I can imagine making it slightly stricter, e.g.
> you could only skip QualifiedNames for inline namespaces and the beginning.
>
rnkovacs accepted this revision.
rnkovacs added a comment.
In https://reviews.llvm.org/D48027#1203944, @MTC wrote:
> However this approach has limit. Given the code below, we cannot distinguish
> whether the `basic_string` is user-defined struct or namespace. That's means
> when the user
xazax.hun added a comment.
Oh, and thanks for working on this, this improvement was long overdue, but
everybody was busy with something else :)
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun accepted this revision.
xazax.hun added a comment.
Sorry for the delay, I think this is OK to commit.
As a possible improvement, I can imagine making it slightly stricter, e.g. you
could only skip QualifiedNames for inline namespaces and the beginning. Maybe
add support for staring
MTC added a comment.
@xazax.hun What do you think?
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
MTC added a comment.
In https://reviews.llvm.org/D48027#1201248, @xazax.hun wrote:
> Generally looks good, I only wonder if this works well with inline
> namespaces. Could you test? Probably it will.
Thank you for reminding me! Yea, this can handle inline namespaces.
However this approach
MTC updated this revision to Diff 161217.
MTC added a comment.
- rebase
- Since we have enhanced the ability of `CallDescription`, remove the helper
method `isCalledOnStringObject()`.
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
xazax.hun added a comment.
Generally looks good, I only wonder if this works well with inline namespaces.
Could you test?
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Szelethus.
This looks great, thanks, this is exactly how i imagined it!
Repository:
rC Clang
https://reviews.llvm.org/D48027
MTC added a comment.
kindly ping!
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
MTC updated this revision to Diff 157499.
MTC added a comment.
@xazax.hun Thanks for your tips! After some investigation, `MatchFinder::match`
just traverse one ASTNode, that means `match(namedDecl(HasNameMatcher()))` and
`match(namedDecl(matchesName()))` both not traverse children. So there
xazax.hun added a comment.
In https://reviews.llvm.org/D48027#1142324, @MTC wrote:
> - There is possible match the wrong AST node, e.g. for the NamedDecl `foo()`,
> if the function body has the `a::b::x`, when we match `a::b::X()`, we will
> match `foo()` . Because I'm not familiar with
MTC added a comment.
Thanks for your review, NoQ!
Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68
: IILockGuard(nullptr), IIUniqueLock(nullptr),
- LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
-
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68
: IILockGuard(nullptr), IIUniqueLock(nullptr),
- LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
- FgetsFn("fgets"), ReadFn("read"),
MTC added a comment.
kindly ping!
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
MTC updated this revision to Diff 152702.
MTC added a comment.
Sorry for the long long delay, I was on the Dragon Boat Festival a few days ago.
This update has two parts:
- Use the `matchesName` to match the AST node with the specified name,
`matchesName` use regex to match the specified name.
MTC added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+
MTC updated this revision to Diff 151166.
MTC added a comment.
- Use `hasName` matcher to match the qualified name.
- Use the full name, like `std::basic_string::c_str` instead of `c_str`
to match the `c_str` method in `DanglingInternalBufferChecker.cpp`.
Repository:
rC Clang
MTC added a comment.
In https://reviews.llvm.org/D48027#1128301, @xazax.hun wrote:
> Having C++ support would be awesome!
> Thanks for working on this!
>
> While I do agree matching is not trivial with qualified names, this problem
> is already solved with AST matchers.
>
> I wonder if using
xazax.hun added a comment.
Having C++ support would be awesome!
Thanks for working on this!
While I do agree matching is not trivial with qualified names, this problem is
already solved with AST matchers.
I wonder if using matchers or reusing the `HasNameMatcher` class would make
this easier.
MTC updated this revision to Diff 150758.
MTC added a comment.
Remove useless header files for testing.
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Core/CallEvent.cpp
Index:
MTC added a comment.
The implementation is not complicated, the difficulty is that there is no good
way to get the qualified name without template arguments. For
'std::basic_string::c_str()', its qualified name may be
`std::__1::basic_string,
std::__1::allocator >::c_str`, it is almost
MTC created this revision.
MTC added reviewers: xazax.hun, NoQ, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, rnkovacs, szepet.
`CallDecription` can only handle function for the time being. If we want to
match c++ method, we can only use method name to match and can't
28 matches
Mail list logo
