[llvm] [SandboxVectorizer][NFCI] Fix use of possibly-uninitialized scalar. (PR #122201)

Jorge Gorbe Moya via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 9 15:28:52 PST 2025


================
@@ -37,10 +38,25 @@ class Context {
   using MoveInstrCallback =
       std::function<void(Instruction *, const BBIterator &)>;
 
-  /// An ID for a registered callback. Used for deregistration. Using a 64-bit
-  /// integer so we don't have to worry about the unlikely case of overflowing
-  /// a 32-bit counter.
-  using CallbackID = uint64_t;
+  /// An ID for a registered callback. Used for deregistration. A dedicated type
+  /// is employed so as to keep IDs opaque to the end user; only Context should
+  /// deal with its underlying representation.
+  class CallbackID {
+  public:
+    // Uses a 64-bit integer so we don't have to worry about the unlikely case
+    // of overflowing a 32-bit counter.
+    using ReprTy = uint64_t;
+
+  private:
+    // Default initialization results in an invalid ID.
+    ReprTy Val = std::numeric_limits<ReprTy>::max();
----------------
slackito wrote:

I think this will conflict with the EmptyKey value used in the map:
```
template<> struct DenseMapInfo<unsigned long long> {
  static inline unsigned long long getEmptyKey() { return ~0ULL; }
  static inline unsigned long long getTombstoneKey() { return ~0ULL - 1ULL; }
...
```

 If we try to unregister a default-initialized callback, we'll do
```
  [[maybe_unused]] bool Erased = EraseInstrCallbacks.erase(ID);
```
and I'm not sure it's correct to call `erase` with the EmptyKey. We could reserve ID 0 as the default value and start assigning callbacks at 1 to avoid this.

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


More information about the llvm-commits mailing list