rnkovacs added inline comments.
================
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}}
----------------
dcoughlin wrote:
> 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".
I feel like I should also retitle the checker to `DanglingInnerBuffer` to
remain consistent. What do you think?
================
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}}
----------------
dcoughlin wrote:
> 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'"
>
>
I like these, thanks! I went with the stronger versions now, as they seem to
fit better with the warnings themselves.
https://reviews.llvm.org/D49360
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
