Skip to content

Conversation

@Priyanshi-dixit1
Copy link

This PR improves the documentation of non-trivial insertion APIs in
llvm::DenseMap.

The updated comments clarify the behaviour of:

  1. try_emplace overloads
  2. insert_or_assign
  3. emplace_or_assign
  4. lookup_or

Specifically, the documentation now explains:

  1. When mapped values are constructed
  2. When existing values are preserved or replaced
  3. behaviour with non-default-constructible mapped types

Addresses #166155.

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2025

@llvm/pr-subscribers-llvm-adt

Author: None (Priyanshi-dixit1)

Changes

This PR improves the documentation of non-trivial insertion APIs in
llvm::DenseMap.

The updated comments clarify the behaviour of:

  1. try_emplace overloads
  2. insert_or_assign
  3. emplace_or_assign
  4. lookup_or

Specifically, the documentation now explains:

  1. When mapped values are constructed
  2. When existing values are preserved or replaced
  3. behaviour with non-default-constructible mapped types

Addresses #166155.


Full diff: https://github.com/llvm/llvm-project/pull/172177.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/DenseMap.h (+54-10)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index fe8868619730e..df214bda2ad1f 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -208,9 +208,16 @@ class DenseMapBase : public DebugEpochBase {
     return ValueT();
   }
 
-  // Return the entry with the specified key, or \p Default. This variant is
-  // useful, because `lookup` cannot be used with non-default-constructible
-  // values.
+  /// Returns the mapped value for the given key, or a provided default.
+  ///
+  /// If the key exists in the map, its mapped value is returned. Otherwise,
+  /// the supplied default value is returned.
+  ///
+  /// This function is useful when the mapped type is not default-constructible.
+  ///
+  /// \param Val The key to look up.
+  /// \param Default The value to return if the key is not found.
+  /// \returns The mapped value or the provided default.
   template <typename U = std::remove_cv_t<ValueT>>
   [[nodiscard]] ValueT lookup_or(const_arg_type_t<KeyT> Val,
                                  U &&Default) const {
@@ -249,17 +256,25 @@ class DenseMapBase : public DebugEpochBase {
     return try_emplace_impl(std::move(KV.first), std::move(KV.second));
   }
 
-  // Inserts key,value pair into the map if the key isn't already in the map.
-  // The value is constructed in-place if the key is not in the map, otherwise
-  // it is not moved.
+  /// Attempts to insert a new element into the map.
+  ///
+  /// If the key does not already exist in the map, a new element is inserted
+  /// and the mapped value is constructed in-place using the provided arguments.
+  ///
+  /// If the key already exists, no insertion is performed and the existing
+  /// mapped value is left unchanged.
+  ///
+  /// \param Key The key to insert.
+  /// \param Args Arguments forwarded to construct the mapped value if insertion
+  /// occurs.
+  /// \returns A pair consisting of an iterator to the element and a boolean
+  ///          indicating whether insertion took place.
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&...Args) {
     return try_emplace_impl(std::move(Key), std::forward<Ts>(Args)...);
   }
 
-  // Inserts key,value pair into the map if the key isn't already in the map.
-  // The value is constructed in-place if the key is not in the map, otherwise
-  // it is not moved.
+   /// \overload
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&...Args) {
     return try_emplace_impl(Key, std::forward<Ts>(Args)...);
@@ -295,6 +310,15 @@ class DenseMapBase : public DebugEpochBase {
     insert(adl_begin(R), adl_end(R));
   }
 
+  /// Inserts a new element or assigns to the existing one.
+  ///
+  /// If the key does not exist, a new element is inserted. If the key already
+  /// exists, the mapped value is replaced with the provided value.
+  ///
+  /// \param Key The key to insert or update.
+  /// \param Val The value to insert or assign.
+  /// \returns A pair consisting of an iterator to the element and a boolean
+  ///          indicating whether insertion took place.
   template <typename V>
   std::pair<iterator, bool> insert_or_assign(const KeyT &Key, V &&Val) {
     auto Ret = try_emplace(Key, std::forward<V>(Val));
@@ -302,7 +326,8 @@ class DenseMapBase : public DebugEpochBase {
       Ret.first->second = std::forward<V>(Val);
     return Ret;
   }
-
+  
+  /// \overload
   template <typename V>
   std::pair<iterator, bool> insert_or_assign(KeyT &&Key, V &&Val) {
     auto Ret = try_emplace(std::move(Key), std::forward<V>(Val));
@@ -311,6 +336,16 @@ class DenseMapBase : public DebugEpochBase {
     return Ret;
   }
 
+  /// Inserts a new element or assigns a newly constructed value.
+  ///
+  /// If the key does not exist, a new element is inserted and the mapped value
+  /// is constructed in-place from the provided arguments. If the key exists,
+  /// the mapped value is replaced.
+  ///
+  /// \param Key The key to insert or update.
+  /// \param Args Arguments used to construct the mapped value.
+  /// \returns A pair consisting of an iterator to the element and a boolean
+  ///          indicating whether insertion took place.
   template <typename... Ts>
   std::pair<iterator, bool> emplace_or_assign(const KeyT &Key, Ts &&...Args) {
     auto Ret = try_emplace(Key, std::forward<Ts>(Args)...);
@@ -319,6 +354,7 @@ class DenseMapBase : public DebugEpochBase {
     return Ret;
   }
 
+  /// \overload
   template <typename... Ts>
   std::pair<iterator, bool> emplace_or_assign(KeyT &&Key, Ts &&...Args) {
     auto Ret = try_emplace(std::move(Key), std::forward<Ts>(Args)...);
@@ -346,10 +382,18 @@ class DenseMapBase : public DebugEpochBase {
     incrementNumTombstones();
   }
 
+  /// Returns a reference to the mapped value for the given key.
+  ///
+  /// If the key does not exist, a new element is inserted with a
+  /// default-constructed mapped value.
+  ///
+  /// \param Key The key to access.
+  /// \returns A reference to the mapped value.
   ValueT &operator[](const KeyT &Key) {
     return lookupOrInsertIntoBucket(Key).first->second;
   }
 
+  /// \overload
   ValueT &operator[](KeyT &&Key) {
     return lookupOrInsertIntoBucket(std::move(Key)).first->second;
   }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants