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

[khuttun]

Summary

Warns on std::map and std::unordered_map lookups done with operator[]. Inserting values with operator[] is still allowed.

[khuttun]

This checker is inspired by Louis Brandy's excellent CppCon 2017 talk (https://www.youtube.com/watch?v=lkgszkPnV8g) where he describes the side-effects of std::map::operator[] as being one of the most common source of bugs in Facebook's C++ code. In the talk he mentions that banning the usage of the operator was discussed at Facebook. This checker bans reading the map with it, writing is still allowed.

The documentation is intentionally still missing from the commit. If the reviewers feel that this sort of checker would be useful for clang-tidy, I'll add the documentation and more tests.

[lebedev.ri]

clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp

25 ↗

(On Diff #144723)

I would imagine it might be best to make this configurable from the start.

[Eugene.Zelenko]

clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp

36 ↗

(On Diff #144723)

Will be good idea to use actual container type.

docs/ReleaseNotes.rst

63 ↗

(On Diff #144723)

Please add short description and place with other new checks in alphabetical order.

docs/clang-tidy/checks/bugprone-map-subscript-operator-lookup.rst

6 ↗

(On Diff #144723)

Please add description and examples.

[aaron.ballman]

I am generally in favor of this but would be curious to see how the check reacts to real world code bases. Facebook's usage experience is compelling, but I'm also wondering how bad the false-positive rate is on existing code. Can you try running the check over LLVM itself (perhaps after adding the option to add custom data types to the list and adding DenseMap and friends) and report back on what you find?

clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp

21 ↗

(On Diff #144723)

No need to register the matchers unless in C++ mode.

clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.h

19 ↗

(On Diff #144723)

Should fix up the description.

docs/clang-tidy/checks/list.rst

96 ↗

(On Diff #144723)

This change looks unrelated (but reasonable -- you can commit this change without review).

[khuttun]

Abandoning this. The false positive rate would be too high for this checker.

[lebedev.ri]

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 :/

[mgehre]

test/clang-tidy/bugprone-map-subscript-operator-lookup.cpp

60 ↗

(On Diff #144723)

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)?

[aaron.ballman]

In 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,

I don't think that's accurate. We still care about false-positive rates and ask people to report on them when we think there's going to be an issue. That's not to say that we shouldn't proceed with this patch.

[khuttun]

In 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.

test/clang-tidy/bugprone-map-subscript-operator-lookup.cpp

60 ↗

(On Diff #144723)

This would not be flagged, as the reference is non-const

[• Quuxplusone]

test/clang-tidy/bugprone-map-subscript-operator-lookup.cpp

48 ↗

(On Diff #144723)

Besides mgehre's request for auto& test cases, I'd like to see some test cases involving these member functions. I assume that's why these lines of code are here (but no tests use them yet).

[khuttun]

The patch is now updated to be based on the monorepo. I also fixed all the comments from the previous review.

[khuttun]

I ran the check for LLVM code using configuration key: bugprone-map-subscript-operator-lookup.MapTypes, value: "::std::map;::std::unordered_map;::llvm::DenseMapBase;::llvm::MapVector;::llvm::StringMap". It gave 1212 warnings (see the attachment).

warnings.txt172 KBDownload

[khuttun]

Documented the MapTypes check option.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp

31

Should be in isLanguageVersionSupported() method.

clang-tools-extra/docs/ReleaseNotes.rst

78

Please separate with empty line.

81

Please synchronize with first statement in documentation.

[khuttun]

Fixed @Eugene.Zelenko's comments

[khuttun]

Accidentally removed the options documentation in the previous patch... Now added it back.

[khuttun]

Any comments on this? Is this checker something that could be part of clang-tidy?

[aaron.ballman]

In D46317#2023406, @khuttun wrote:

Any comments on this? Is this checker something that could be part of clang-tidy?

Thank you for posting some of the diagnostics found by the check, that was really helpful information! I spot-checked ~10 of the issues it reported and all of them were false positives. Were you able to find any true positives from that list? I think 1200 reports without any true positives indicates that the check may be too chatty to include (it may also suggest that bugprone is the wrong place for the check).

[khuttun]

In D46317#2027071, @aaron.ballman wrote:

In D46317#2023406, @khuttun wrote:

Any comments on this? Is this checker something that could be part of clang-tidy?

Thank you for posting some of the diagnostics found by the check, that was really helpful information! I spot-checked ~10 of the issues it reported and all of them were false positives. Were you able to find any true positives from that list? I think 1200 reports without any true positives indicates that the check may be too chatty to include (it may also suggest that bugprone is the wrong place for the check).

It's difficult to spot actual functionality bugs without knowing the code better, but there's plenty of unnecessary double-lookups (count()/find() + operator[]) reported, for example:

clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp:75:30
clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp:157:33
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:106:29

[aaron.ballman]

In D46317#2027330, @khuttun wrote:

In D46317#2027071, @aaron.ballman wrote:

In D46317#2023406, @khuttun wrote:

Any comments on this? Is this checker something that could be part of clang-tidy?

Thank you for posting some of the diagnostics found by the check, that was really helpful information! I spot-checked ~10 of the issues it reported and all of them were false positives. Were you able to find any true positives from that list? I think 1200 reports without any true positives indicates that the check may be too chatty to include (it may also suggest that bugprone is the wrong place for the check).

It's difficult to spot actual functionality bugs without knowing the code better, but there's plenty of unnecessary double-lookups (count()/find() + operator[]) reported, for example:

Definitely agreed that it's labor-intensive. :-)

clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp:75:30
clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp:157:33
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:106:29

I hadn't spot checked those, but did see at least one similar case with what I checked. However, I consider those false positives because none of these are cases where the code gives wrong results. It is possible they are performance concerns (though I'd be curious whether these patterns optimize well or not), so if the check was specific to finding that scenario, that would be pretty useful (though it likely requires codeflow analysis).

[lbrandy]

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.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "ForwardingReferenceOverloadCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncorrectRoundingsCheck.h"
 #include "InfiniteLoopCheck.h"
 #include "IntegerDivisionCheck.h"
 #include "LambdaFunctionNameCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
+#include "MapSubscriptOperatorLookupCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "MisplacedPointerArithmeticInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<IntegerDivisionCheck>(
         "bugprone-integer-division");
     CheckFactories.registerCheck<LambdaFunctionNameCheck>(
         "bugprone-lambda-function-name");
     CheckFactories.registerCheck<MacroParenthesesCheck>(
         "bugprone-macro-parentheses");
     CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
         "bugprone-macro-repeated-side-effects");
+    CheckFactories.registerCheck<MapSubscriptOperatorLookupCheck>(
+        "bugprone-map-subscript-operator-lookup");
     CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>(
         "bugprone-misplaced-operator-in-strlen-in-alloc");
     CheckFactories.registerCheck<MisplacedPointerArithmeticInAllocCheck>(
         "bugprone-misplaced-pointer-arithmetic-in-alloc");
     CheckFactories.registerCheck<MisplacedWideningCastCheck>(
         "bugprone-misplaced-widening-cast");
     CheckFactories.registerCheck<MoveForwardingReferenceCheck>(
         "bugprone-move-forwarding-reference");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   ForwardingReferenceOverloadCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectRoundingsCheck.cpp
   InfiniteLoopCheck.cpp
   IntegerDivisionCheck.cpp
   LambdaFunctionNameCheck.cpp
   MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp
+  MapSubscriptOperatorLookupCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
   MisplacedPointerArithmeticInAllocCheck.cpp
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
   NotNullTerminatedResultCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp

clang-tools-extra/clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.h

+//===--- MapSubscriptOperatorLookupCheck.h - clang-tidy ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MAPSUBSCRIPTOPERATORLOOKUPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MAPSUBSCRIPTOPERATORLOOKUPCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Detects map lookups done with operator[].
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-map-subscript-operator-lookup.html
+class MapSubscriptOperatorLookupCheck : public ClangTidyCheck {
+public:
+  MapSubscriptOperatorLookupCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  std::string MapTypes;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MAPSUBSCRIPTOPERATORLOOKUPCHECK_H

clang-tools-extra/clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp

+//===--- MapSubscriptOperatorLookupCheck.cpp - clang-tidy -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MapSubscriptOperatorLookupCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+MapSubscriptOperatorLookupCheck::MapSubscriptOperatorLookupCheck(
+    llvm::StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      MapTypes(Options.get("MapTypes", "::std::map;::std::unordered_map")) {}
+
+void MapSubscriptOperatorLookupCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "MapTypes", MapTypes);
+}
+
+void MapSubscriptOperatorLookupCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  auto MapTypeVec = utils::options::parseStringList(MapTypes);
+  Finder->addMatcher(
+      cxxOperatorCallExpr(
+          callee(cxxMethodDecl(hasName("operator[]"),
+                               ofClass(hasAnyName(std::vector<StringRef>(
+                                   MapTypeVec.begin(), MapTypeVec.end()))))
+                     .bind("operator")),
+          anyOf(
+              // the return value is used in const context
+              hasParent(implicitCastExpr(
+                  anyOf(hasImplicitDestinationType(isConstQualified()),
+                        // for scalar types const usage of the return value is
+                        // seen as lvalue -> rvalue cast
+                        hasCastKind(CK_LValueToRValue)))),
+              // address of the return value is taken, but the resulting pointer
+              // is used as pointer-to-const
+              hasParent(unaryOperator(
+                  hasOperatorName("&"),
+                  hasParent(implicitCastExpr(hasImplicitDestinationType(
+                      pointsTo(isConstQualified()))))))))
+          .bind("call"),
+      this);
+}
+
+void MapSubscriptOperatorLookupCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
+  const auto *Operator = Result.Nodes.getNodeAs<CXXMethodDecl>("operator");
+  diag(Call->getBeginLoc(), "do not use operator[] for %0 lookup")
+      << Operator->getParent()->getName();
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 New module
 ^^^^^^^^^^
 - New module `llvmlibc`.
 
   This module contains checks related to the LLVM-libc coding standards.
 
 New checks
 ^^^^^^^^^^
+- New :doc:`bugprone-map-subscript-operator-lookup
+  <clang-tidy/checks/bugprone-map-subscript-operator-lookup>` check.
+
+  Detects map lookups done with operator[].
+
 - New :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables>` check.
   Finds non-const global variables as described in check I.2 of C++ Core
   Guidelines.
 
 - New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
   <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone-map-subscript-operator-lookup.rst

+.. title:: clang-tidy - bugprone-map-subscript-operator-lookup
+
+bugprone-map-subscript-operator-lookup
+======================================
+
+Using ``std::map::operator[]`` for finding values from the map is a common
+source of bugs in C++ programs. The operator's side effect of possibly inserting
+a value to the map can easily cause surprises. The usage of the operator can be
+replaced with ``at()`` or ``find()`` in cases where it's used to only look up
+values from the map with no intention of modifying it.
+
+This check warns when the operator is used for map lookup:
+
+.. code-block:: c++
+
+  std::map<int, int> m;
+  const auto i = m[1]; // warning
+
+The check only warns about *lookups* done with ``operator[]``. Modifying the map
+using it is still ok:
+
+.. code-block:: c++
+
+  std::map<int, int> m;
+  m[1] = 2; // ok
+
+In general, doing any mutating operation through the reference returned by the
+operator is allowed by this check. If the reference is used as
+reference-to-const, a warning is given:
+
+.. code-block:: c++
+
+  std::map<int, Foo> m;
+  m[1].setter(1); // ok
+  const auto a = m[1].getter(); // warning
+
+Options
+-------
+
+.. option:: MapTypes
+
+   Semicolon-separated list of map types to check. Defaults to
+   ``::std::map;::std::unordered_map``.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-forwarding-reference-overload <bugprone-forwarding-reference-overload.html>`_,
    `bugprone-inaccurate-erase <bugprone-inaccurate-erase.html>`_, "Yes"
    `bugprone-incorrect-roundings <bugprone-incorrect-roundings.html>`_,
    `bugprone-infinite-loop <bugprone-infinite-loop.html>`_,
    `bugprone-integer-division <bugprone-integer-division.html>`_,
    `bugprone-lambda-function-name <bugprone-lambda-function-name.html>`_,
    `bugprone-macro-parentheses <bugprone-macro-parentheses.html>`_, "Yes"
    `bugprone-macro-repeated-side-effects <bugprone-macro-repeated-side-effects.html>`_,
+   `bugprone-map-subscript-operator-lookup <bugprone-map-subscript-operator-lookup.html>`_,
    `bugprone-misplaced-operator-in-strlen-in-alloc <bugprone-misplaced-operator-in-strlen-in-alloc.html>`_, "Yes"
    `bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone-misplaced-pointer-arithmetic-in-alloc.html>`_, "Yes"
    `bugprone-misplaced-widening-cast <bugprone-misplaced-widening-cast.html>`_,
    `bugprone-move-forwarding-reference <bugprone-move-forwarding-reference.html>`_, "Yes"
    `bugprone-multiple-statement-macro <bugprone-multiple-statement-macro.html>`_,
    `bugprone-not-null-terminated-result <bugprone-not-null-terminated-result.html>`_, "Yes"
    `bugprone-parent-virtual-call <bugprone-parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone-posix-return.html>`_, "Yes"

clang-tools-extra/test/clang-tidy/checkers/bugprone-map-subscript-operator-lookup-custom.cpp

+// RUN: %check_clang_tidy %s bugprone-map-subscript-operator-lookup %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: bugprone-map-subscript-operator-lookup.MapTypes, \
+// RUN:    value: "::ns::Map"}]}' \
+// RUN: --
+
+namespace ns {
+
+template <typename K, typename V>
+struct Map {
+  V &operator[](const K &);
+  V &operator[](K &&);
+};
+
+template <typename K, typename V>
+struct DerivedMap : public Map<K, V> {};
+
+using IntMap = Map<int, int>;
+
+} // namespace ns
+
+void warning() {
+  ns::Map<int, int> M;
+  auto A = M[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: do not use operator[] for Map lookup [bugprone-map-subscript-operator-lookup]
+
+  ns::DerivedMap<int, int> DM;
+  auto B = DM[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: do not use operator[] for Map lookup [bugprone-map-subscript-operator-lookup]
+
+  ns::IntMap IM;
+  auto C = IM[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: do not use operator[] for Map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void noWarning() {
+  ns::Map<int, int> MOk;
+  MOk[1] = 2;
+
+  ns::DerivedMap<int, int> DMOk;
+  DMOk[1] = 2;
+
+  ns::IntMap IMOk;
+  IMOk[1] = 2;
+}

clang-tools-extra/test/clang-tidy/checkers/bugprone-map-subscript-operator-lookup.cpp

+// RUN: %check_clang_tidy %s bugprone-map-subscript-operator-lookup %t
+
+namespace std {
+
+template <typename T>
+struct allocator {};
+
+template <typename T>
+struct equal_to {};
+
+template <typename T>
+struct hash {};
+
+template <typename T>
+struct less {};
+
+template <typename T, typename U>
+struct pair {};
+
+template <typename Key,
+          typename T,
+          typename Compare = std::less<Key>,
+          typename Allocator = std::allocator<std::pair<const Key, T>>>
+struct map {
+  T &operator[](const Key &);
+  T &operator[](Key &&);
+};
+
+// the check should be able to match std lib calls even if the functions are
+// declared inside inline namespaces
+inline namespace v1 {
+
+template <typename Key,
+          typename T,
+          typename Hash = std::hash<Key>,
+          typename KeyEqual = std::equal_to<Key>,
+          typename Allocator = std::allocator<std::pair<const Key, T>>>
+struct unordered_map {
+  T &operator[](const Key &);
+  T &operator[](Key &&);
+};
+
+} // namespace v1
+} // namespace std
+
+struct Foo {
+  int getter() const;
+  void setter(int);
+  bool operator==(const Foo &) const;
+};
+
+using FooPtr = Foo *;
+
+void readInt(const int &);
+void writeInt(int &);
+void copyInt(int);
+
+void readFoo(const Foo &);
+void writeFoo(Foo &);
+void copyFoo(Foo);
+
+void copyFooPtr(Foo *);
+void copyConstFooPtr(const Foo *);
+
+std::map<int, int> IntMap;
+std::map<int, Foo> FooMap;
+std::unordered_map<int, FooPtr> FooPtrMap;
+
+void copyFromMap() {
+  int A = IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  Foo B = FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  FooPtr C = FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void constRefFromMap() {
+  const int &A = IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:18: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  const Foo &B = FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:18: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  const FooPtr &C = FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void constPtrFromMap() {
+  const int *A = &IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  const Foo *B = &FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  const FooPtr *C = &FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:22: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+int returnInt() {
+  return IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+Foo returnFoo() {
+  return FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+FooPtr returnFooPtr() {
+  return FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+const int &returnRefToConstInt() {
+  return IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+const Foo &returnRefToConstFoo() {
+  return FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+const FooPtr &returnRefToConstFooPtr() {
+  return FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+const int *returnPtrToConstInt() {
+  return &IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+const Foo *returnPtrToConstFoo() {
+  return &FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+const FooPtr *returnPtrToConstFooPtr() {
+  return &FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void callReadX() {
+  readInt(IntMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  readFoo(FooMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void callCopyX() {
+  copyInt(IntMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  copyFoo(FooMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  copyFooPtr(FooPtrMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+  copyConstFooPtr(FooPtrMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void passAddrToFunctionTakingConstPtr() {
+  copyConstFooPtr(&FooMap[1]);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void callConstMemf() {
+  FooMap[1].getter();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void deref() {
+  Foo F = *FooPtrMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+  FooPtrMap[1]->getter();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+  FooPtrMap[1]->setter(1);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use operator[] for unordered_map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void arithmetic() {
+  int A = 1 + IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void comparison() {
+  bool A = IntMap[1] == 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  bool B = FooMap[1] == Foo{};
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void convertedToBool() {
+  if (IntMap[1])
+    // CHECK-MESSAGES: [[@LINE-1]]:7: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+    ;
+}
+
+void thrown() {
+  throw IntMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+  throw FooMap[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: do not use operator[] for map lookup [bugprone-map-subscript-operator-lookup]
+}
+
+void writeToMap() {
+  IntMap[1] = 1;
+  FooMap[1] = Foo{};
+  FooPtrMap[1] = new Foo;
+}
+
+void refFromMap() {
+  int &A = IntMap[1];
+  Foo &B = FooMap[1];
+  FooPtr &C = FooPtrMap[1];
+}
+
+void ptrFromMap() {
+  int *A = &IntMap[1];
+  Foo *B = &FooMap[1];
+  FooPtr *C = &FooPtrMap[1];
+}
+
+int &returnRefToInt() {
+  return IntMap[1];
+}
+
+Foo &returnRefToFoo() {
+  return FooMap[1];
+}
+
+FooPtr &returnRefToFooPtr() {
+  return FooPtrMap[1];
+}
+
+int *returnPtrToInt() {
+  return &IntMap[1];
+}
+
+Foo *returnPtrToFoo() {
+  return &FooMap[1];
+}
+
+FooPtr *returnPtrToFooPtr() {
+  return &FooPtrMap[1];
+}
+
+void callWriteX() {
+  writeInt(IntMap[1]);
+  writeFoo(FooMap[1]);
+}
+
+void passAddrToFunctionTakingPtr() {
+  copyFooPtr(&FooMap[1]);
+}
+
+void callMemf() {
+  FooMap[1].setter(1);
+}
+
+void increment() {
+  ++IntMap[1];
+  IntMap[1]++;
+}
+
+void unused() {
+  IntMap[1];
+  FooMap[1];
+  FooPtrMap[1];
+}