[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D113148#3130779 <https://reviews.llvm.org/D113148#3130779>, @whisperity wrote:

> In D113148#3109190 <https://reviews.llvm.org/D113148#3109190>, @CJ-Johnson wrote:
>
>> In D113148#3108993 <https://reviews.llvm.org/D113148#3108993>, @aaron.ballman wrote:
>>
>>> Generally speaking, we prefer to improve the existing checks. I think `bugprone-string-constructor` would probably be a better place for the constructor-related functionality. But that still means we don't have a good place for things like the assignment and comparison functionality, and it seems more useful to keep all of the `string_view`-with-`nullptr` logic in one place. That said, we should be careful we're not too onerous when users enable all `bugprone` checks together.
>>
>> As for "we should be careful we're not too onerous when users enable all `bugprone` checks together.", these fixes are about preventing UB. While I did put this in the "bugprone" module, perhaps "prone" is the wrong way to think about it. It's unconditionally UB in all cases, not just a potential bug. The suggested fixes are important for preventing crashes in the short term, but more importantly for allowing the code to build in the future, since the constructor overload `basic_string_view(nullptr_t) = delete;` is being added.
>
> @aaron.ballman Maybe we should do something to ease the burden of `bugprone-` becoming a catch-all basket, and create a new checker group for the "unconditional UB" or "in almost all cases this will make you do bad things" kinds of stuff.

`bugprone` was originally added because `misc` was becoming too much of a catch-all basket and we wanted a new group for the "doing this is a bug" kind of checks, so there's precedent for splitting groups when we need finer granularity. However, we had quite a few checks that could be moved from `misc` to `bugrone` during that switch. If we added a group for unconditional UB (or morally similar), it'd be good to know how many checks we think need to move and what sort of checks we envision the new module encouraging in the future. That'll help inform us whether it's good value to make a split or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148