[llvm] [DebugInfo] Swap 'Unit' and 'Type' positions in DISubprogram. (PR #96474)

Abid Qadeer via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 26 04:29:44 PDT 2024


abidh wrote:

> Aaaaahhh, I get it now, thanks for bearing with me -- I see that the `Verifier` class has a `CurrentSourceLang` field that gets populated while exploring the metadata tree, and visiting the Unit before the Type in a subprogram ensures that `CurrentSourceLang` gets set before further exploration occurs.
> 
> I still feel flipping the order of fields is fixing the symptom rather than the cause -- stepping back a bit, by having state in the Verifier object we're implicitly requiring metadata to be explored in a particular order. This patch will fix one incorrect order, but there might be other incorrect orders or it might expose more.
> 
> Instead: how about setting CurrentSourceLang in `visitDISubprogram` instead, skipping that behaviour if the Unit appears to be invalid. Whenever we visit a subprogram, we're about to visit a collection of variables, types and other entities that may depend on the source language, and verifying a `Function` independently is a fairly common use-case IMHO. This also avoids relying on an implementation detail of the DISubprogram class layout.
> 
> I get the feeling that the state in `Verifier` will cause trouble in LTO contexts where there are multiple compile units in a `Module`, but I'm not familiar enough with metadata in LTO contexts to give an opinion.

Thanks for taking a look. Setting `CurrentSourceLang` in `visitDISubprogram` is a good suggestion. I have changed the PR to use it. If it looks ok then I will update the heading and description of the PR accordingly.

https://github.com/llvm/llvm-project/pull/96474


More information about the llvm-commits mailing list