khuttun marked an inline comment as done.
khuttun added a comment.
In D46317#1988261 <https://reviews.llvm.org/D46317#1988261>, @lebedev.ri wrote:
> IMHO we should proceed with this patch.
> There's been several patches recently that seem to not care about
> false-positive rate,
> and in this case the cost of true-positive can be humongous, as i/we've been
> finding
> lots of bugs exactly like this in openmp optimization pass, attributor
> framework :/
When developing this checker I tested it on LLVM code base, and there was
hundreds of warnings. Looking at my notes from then, the ones that I analyzed
fell in to couple of different categories:
- The program logic is such that it's known that the key is found in the map
(in some of these cases `count()` is asserted before the lookup)
- The code does `find()` vs.` end()` comparison before the lookup
From these the at least the first category could be considered false positives.
I also think that this checker could bring value, though. For example when
enabled on a new project from the start. I should have time to finish it if
reviewers feel that we should reconsider it. Also, go ahead and share if you
have any ideas on how to make the checker more accurate.
================
Comment at: test/clang-tidy/bugprone-map-subscript-operator-lookup.cpp:60
+}
+
+void noWarning() {
----------------
mgehre wrote:
> We often have code like
> ```
> auto &V = Map[Idx];
> if (!V) {
> V = init();
> }
> use(V);
> ```
> Would that be flagged by this check? Could you add a test for it (possibly
> with `FIXME`)?
This would not be flagged, as the reference is non-const
Repository:
rCTE Clang Tools Extra
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
