[PATCH] D143976: [ADT] Add lookupOrTrap method to DenseMap

Chris Lattner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 13 20:56:53 PST 2023


lattner added a comment.

Very cool, I'm excited to get this.  This will simplify a lot of code!   Could you also add this to StringMap?  It has a similar lookup method but it can't be used for non-default-constructible values, and has the same usage pattern as this container.



================
Comment at: llvm/include/llvm/ADT/DenseMap.h:213
+    dbgs() << "[DenseMap::lookupOrTrap] key not found; " << Msg << "\n";
+    std::abort();
+  }
----------------
Unfortunately, I don't think we can afford to include this level of debug tracing in here.  This adds a bunch of #include dependencies, and (more problematically) includes a bunch of code in the non assert build.

The standard LLVM way to do this (see in things like ArrayRef's subscript) is to just assert with a fixed message "lookupOrTrap failed due to a missing key", using the standard `assert` macro.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143976/new/

https://reviews.llvm.org/D143976



More information about the llvm-commits mailing list