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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits