aaron.ballman added a comment.

In D71686#1794360 <https://reviews.llvm.org/D71686#1794360>, @0x8000-0000 wrote:

> In D71686#1794330 <https://reviews.llvm.org/D71686#1794330>, @aaron.ballman 
> wrote:
>
> > In D71686#1794053 <https://reviews.llvm.org/D71686#1794053>, @0x8000-0000 
> > wrote:
> >
> > > My take: this change fixes a user-reported bug, and does not cause any 
> > > known regressions. I think we should integrate this.
> >
> >
> > I sort of wonder whether we want to document this as a blessed approach to 
> > silencing the warning. I'm not certain if it's too obtuse or not, but I 
> > notice the check has no documented ways to silence the diagnostic aside 
> > from using the correct kind of magic number or adding it to a list of 
> > excluded magic numbers.
>
>
> You mean Hyrum's Law <https://www.hyrumslaw.com/> is not sufficient?
>
> The check can be silenced with the regular NOLINT, or with defining and using 
> a constant/enum. Using this "backdoor" way seems even more cumbersome and 
> confusing than the NOLINT. At least with NOLINT it is clear what you're 
> doing, and somebody else can grep for it and fix it if it is appropriate.


My concern is that `NOLINT` is insufficient. Consider: `foo(12, 42, 18);` where 
the `42


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

https://reviews.llvm.org/D71686



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to