[PATCH] D94640: adds more checks to -Wfree-nonheap-object

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:10314
+  switch (kind) {
+  case clang::CK_IntegralToPointer: // [[fallthrough]];
+  case clang::CK_FunctionToPointerDecay: {
----------------
cjdb wrote:
> aaron.ballman wrote:
> > 
> Done, but why? I quite like making it clear that fallthrough is intentional.
I don't think an explicit marker is necessary because the two cases have no whitespace separating them (so it should be obvious from context that fallthrough is intentional rather than accidental). However, we have the `LLVM_FALLTHROUGH` macro for the cases where fallthrough is intentional -- we typically only use that macro when compilers would otherwise diagnose the fallthrough though.


================
Comment at: clang/test/Analysis/free.c:84
+  // expected-warning at -1{{Argument to free() is a block, which is not memory allocated by malloc()}}
+  // expected-warning at -2{{attempt to call free on non-heap object : block expression}}
 }
----------------
cjdb wrote:
> aaron.ballman wrote:
> > The formatting for this diagnostic is somewhat unfortunate in that it has the leading space before the `:`. I think that changing the diagnostic to use a `%select` would be an improvement.
> I'm having a *lot* of difficulty getting `%select` to work. Here's what I've tried, but the space in `%select{ %2` is being ignored :(
> 
> ```
> : Warning<"attempt to call %0 on non-heap object%select{ %2|: block expression}1">,
> ```
We could cheat a little bit. :-D

`Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression}1">`

(The diagnostic should probably be updated to distinguish between block expressions and lambda expressions, which may add another layer of `%select` not shown here.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94640/new/

https://reviews.llvm.org/D94640