dcoughlin added a comment.
It is really nice to see this checker take shape! Some drive by diagnostic
comments in line.
================
Comment at: test/Analysis/dangling-internal-buffer.cpp:175
std::string s;
- {
- c = s.c_str();
- }
- consume(c); // no-warning
+ c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained
here}}
+ s.clear(); // expected-note {{Method call is allowed to invalidate the
internal buffer}}
----------------
In other parts of clang we use the term "inner pointer" to mean a pointer that
will be invalidated if its containing object is destroyed
https://clang.llvm.org/docs/AutomaticReferenceCounting.html#interior-pointers.
There are existing attributes that use this term to specify that a method
returns an inner pointer.
I think it would be good to use the same terminology here. So the diagnostic
could be something like "Dangling inner pointer obtained here".
================
Comment at: test/Analysis/dangling-internal-buffer.cpp:176
+ c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained
here}}
+ s.clear(); // expected-note {{Method call is allowed to invalidate the
internal buffer}}
+ consume(c); // expected-warning {{Use of memory after it is freed}}
----------------
What do you think about explicitly mentioning the name of the method here when
we have it? This will make it more clear when there are multiple methods on the
same line.
I also think that instead of saying "is allowed to" (which raises the question
"by whom?") you could make it more direct.
How about:
"Inner pointer invalidated by call to 'clear'"
or, for the destructor "Inner pointer invalidated by call to destructor"?
What do you think?
If you're worried about this wording being to strong, you could weaken it with
a "may be" to:
"Inner pointer may be invalidated by call to 'clear'"
https://reviews.llvm.org/D49360
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
