[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 2 11:49:03 PST 2025


================
@@ -0,0 +1,147 @@
+.. title:: clang-tidy - performance-redundant-lookup
+
+performance-redundant-lookup
+============================
+
+This check warns about potential redundant container lookup operations within
+a function.
+
+Examples
+--------
+
+.. code-block:: c++
+
+    if (map.count(key) && map[key] < threshold) {
+      // do stuff
+    }
+
+In this example, we would check if the key is present in the map,
+and then do a second lookup to actually load the value.
+We could refactor the code into this, to use a single lookup:
+
+.. code-block:: c++
+
+    if (auto it = map.find(key); it != map.end() && it->second < threshold) {
+      // do stuff
+    }
+
+In this example, we do three lookups while calculating, caching and then
+using the result of some expensive computation:
+
+.. code-block:: c++
+
+    if (!cache.contains(key)) {
+        cache[key] = computeExpensiveValue();
+    }
+    use(cache[key]);
+
+We could refactor this code using ``try_emplace`` to fill up the cache entry
+if wasn't present, and just use it if we already computed the value.
+This way we would only have a single unavoidable lookup:
+
+.. code-block:: c++
+
+    auto [cacheSlot, inserted] cache.try_emplace(key);
+    if (inserted) {
+        cacheSlot->second = computeExpensiveValue();
+    }
+    use(cacheSlot->second);
+
+
+What is a "lookup"?
+-------------------
+
+All container operations that walk the internal structure of the container
+should be considered as "lookups".
+This means that checking if an element is present or inserting an element
+is also considered as a "lookup".
+
+For example, ``contains``, ``count`` but even the ``operator[]``
+should be considered as "lookups".
+
+Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups,
+even if due to limitations, they are not recognized as such.
+
+Lookups inside macros are ignored, thus not considered as "lookups".
+For example:
+
+.. code-block:: c++
+
+    assert(map.count(key) == 0); // Not considered as a "lookup".
+
+Options
+-------
+
+.. option:: ContainerNameRegex
+
+   The regular expression matching the type of the container objects.
+   This is matched in a case insensitive manner.
+   Default is ``set|map``.
----------------
steakhal wrote:

In that case it wouldnt match `llvm::SetVector`.

https://github.com/llvm/llvm-project/pull/125420


More information about the cfe-commits mailing list