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@-1{{Argument to free() is a block, which is not memory 
allocated by malloc()}}
+  // expected-warning@-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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to