lbrandy added a comment.

Someone pointed this patch/thread out to me so I wanted to give some background 
on what prompted the mention of this in my talk.

I think this particular bug is a _huge_ and _recurring_ problem (we've had yet 
more examples recently, internally) but I also believe that the legitimate uses 
of `map::operator[]` far outnumber the problematic cases. Data would sway me 
but my guess is that the false positive rate of this check would make it 
exceptionally difficult to ever enable this in anger. I don't think this is 
something we (facebook) could or would turn on, by default, in our codebase. I 
mentioned we'd had people seriously discuss banning it but none of those 
arguments actually turned into a consensus to act. I'd be super curious if 
anyone had experience actually trying to use a rule like this in their 
codebase. I just didn't want ya'll to have the impression that internal FB 
guidance and/or rules enforce this successfuly. We don't. And the reason we 
don't is the same reasons expressed above: that the signal-to-noise ratio seems 
way, way off. Obviously that doesn't mean a rule like this for clang-tidy 
wouldn't be useful, maybe it is, but it's a tough rule for me to believe would 
be a good idea to have enabled by default.


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

https://reviews.llvm.org/D46317



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

Reply via email to