[PATCH] D104827: [DebugInfo] Enforce implicit constraints on `distinct` MDNodes

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D104827#3137807 <https://reviews.llvm.org/D104827#3137807>, @scott.linder wrote:

>> Is this moving in the right direction? Does the restriction on DIArgList seem
>> reasonable, and if so does it need more explicit documentation or more thorough
>> enforcement?
>
> I pushed this before looking at the intervening comments, so the last bit is a bit redundant; it sounds like the restriction on DIArgList was intended and makes sense, so I think the remaining question is whether the Verifier check is sufficient.

It seems like `DIArgList` is very similar to LocalAsMetadata. I feel like it'd be less error-prone for it to use/inherit-from `ReplaceableMetadataImpl`, just like ValueAsMetadata does. Then it has the machinery to RAUW/delete itself when necessary.

(Moreover, given that it doesn't have any operands, I find it suspicious that it inherits (albeit indirectly) from `MDNode`. It should probably be a direct subclass of `Metadata`. This would mean it couldn't inherit from `DINode` which might complicate some debug info code; if you're going to clean that up I suggest doing it later as a separate patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104827