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

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Thanks!



================
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());
----------------
jmorse wrote:
> 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
I can't seem to edit inline comments. For posterity: in the first sentence, when I said "on a function" I meant "on an instruction".


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

https://reviews.llvm.org/D132224