[clang] [clang][analyzer] Simplify code of StreamChecker (NFC). (PR #79312)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 24 09:43:43 PST 2024


steakhal wrote:

I'm yet to review the PR, but I would express my opinion on the ergonomics of the StreamChecker, as I've spent the last couple of days around it.

I find code duplication less harmful than unnatural generalization over small set of functions (I know, it's a hot take :D).
`std::bind` is harmful, and specific handlers should be preferred. As a rule of thumb, if we need a flag on a top-level API, then that's likely a code-smell.

E.g.: There presence of `IsRead` or `IsSingleChar` (that we need to bind), signals to me that there should be a specific API for the read, write, singleChar and buffer cases. Internally, those could dispatch to a generic call, which has this flag - but at least on the use-sites, we don't need to use `std::bind`, but naturally just taking the address of the member function without any tricks.

I'd also advise against using more callables bundled with CallDescriptions. They make debugging code more difficult, as the control-flow would become also data-dependent. I'd suggest other ways to generalize.
If I have some spare time in the next days, I'm going to also look deeper and come up with actual recommendations.

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


More information about the cfe-commits mailing list