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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: alexfh, njames93, hokein, whisperity.
aaron.ballman added a comment.
Herald added a subscriber: rnkovacs.

In D113148#3108960 <https://reviews.llvm.org/D113148#3108960>, @Sockke wrote:

> In D113148#3108705 <https://reviews.llvm.org/D113148#3108705>, @CJ-Johnson wrote:
>
>> In D113148#3107897 <https://reviews.llvm.org/D113148#3107897>, @Sockke wrote:
>>
>>> This seems to be an existing check. Have you compared it with **bugprone-string-constructor**?
>>
>> Thanks for the suggestion! From what I can tell, bugprone-string-constructor check only has warnings and does not provide fixes in most cases. The goal of bugprone-stringview-nullptr is to robustly enumerate the many cases that it cares about and provide fixes. For that reason, I think making it a separate check is best.
>
> Yes, But i think that improving existing check is the best way. Because improving bugprone-string-construct in a new check may make developers confused and cause redundant overlap.
> Let's see if @aaron.ballman or @whisperity has any comments?

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148