[PATCH] D132224: [Assignment Tracking][5/*] Add core infrastructure for instruction reference

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/IR/DebugInfo.cpp:1646-1649
+  // The ID is only used wrapped in MetadataAsValue(ID), so lets check that
+  // one of those already exists first.
+  if (!IDAsValue)
+    return make_range(Value::user_iterator(), Value::user_iterator());
----------------
Orlando wrote:
> jmorse wrote:
> > Depending on how this API is to be used, would it be better to assert that an existing DIAssignID is always used? "Fail fast fail hard" works pretty well to detect deep errors.
> This isn't a error state - you can get into the situation where you have a `DIAssignID` attachment on a function which is not used by any `llvm.dbg.assign` intrinsics. For instance, if a `llvm.dbg.assign` has been deleted due to living in a dead block -- whether or not _that_ is desirable needs to be looked at on a case-by-case basis IMO, so an assert would be too broad.
SGTM


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

https://reviews.llvm.org/D132224