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