[clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)

Bill Wendling via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 19 16:43:34 PST 2024


bwendling wrote:

> Hmm, I don’t think this is the right way of going about it—copy-pasting the check across 6 or so different places seems like it’s missing the point of using a placeholder in the first place.

Could you point to a place in the code where it creates a placeholder? I'm unable to find any others that what I modified in SemaExpr.cpp. Many of the uses you mentioned would be invalid to perform on other `__builtin` functions didn't produce an error when I tried them out (except for using the `AddrOf` operator).

> Maybe I didn’t describe this accurately enough, but the idea was that the _call_ to the builtin, not the builtin itself, should get placeholder type. Then, you need to update `CheckPlaceholderExpr` _only_ (and maybe the initialisation code if it’s required there; I don’t remember if that ever checks for placeholders off the top of my head because initialisers are weird, but I hope it does), and then update any places where you want to _allow_ this as a subexpression to check for an expression w/ that placeholder type (and change the placeholder type to be whatever the actual type of the expression is supposed to be) before it calls `CheckPlaceholderExpr`/`CheckArgsForPlaceholders`.

Given that I did this by checking the `CallExpr`, perhaps we should just abandon using the placeholder scheme and go with what I have here, but without the placeholder stuff.

https://github.com/llvm/llvm-project/pull/116719


More information about the cfe-commits mailing list