[PATCH] D152095: [Verifier] definition subprograms cannot be nested within DICompositeType when enabling ODR.

Wed Jul 5 11:57:02 PDT 2023

dblaikie added a comment.

In D152095#4459678 <https://reviews.llvm.org/D152095#4459678>, @DianQK wrote:

> In D152095#4457017 <https://reviews.llvm.org/D152095#4457017>, @dblaikie wrote:
>
>> Not sure I follow - I guess regardless they probably shouldn't be in this change.
>
> I want to remove these two IRs from this change. Since these violate the current convention, these don't make sense anymore.

I'm still not quite following why this patch has changes to `llvm/lib/CodeGen/AsmPrinter`? Is the point that those changes can be removed once this invariant is introduced? If so, those changes should probably be in a follow-up after this one, rather than at the same time.

>> I'd expect this change to add the verifier check, and add an example IR that doesn't pass verification/demonstrates the new verifier check correctly catches the intended case(s).
>
> This should be `llvm/test/Verifier/disubprogram-declaration-within-struct.ll`?

Yep, that looks OK. Though I don't understand the phrasing "when enabling ODR" - what does that refer to? (is there some feature to "enable/disable ODR" that's relevant here?)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152095