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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
