[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

[Louis Brandy via Phabricator via cfe-commits]

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