[PATCH] D106615: [Clang][LLVM] generate btf_tag annotations for DIComposite types

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song added a comment.

In D106615#2951101 <https://reviews.llvm.org/D106615#2951101>, @dblaikie wrote:

> In D106615#2950964 <https://reviews.llvm.org/D106615#2950964>, @yonghong-song wrote:
>
>> The attributes should be revolved during semantic analysis stage. So looks like replace-style attribute setting is not really needed. I will change to add attribute parameter during creation time. Thanks for suggestion.
>
> I think semantic analysis runs alongside code generation - eg, if you have one attribute applied to a declaration, then a function definition using that type, then another declaration of the same type, but with a different attribute, then another function definition using the type - if you build the type only once, on the first use, you might not capture all the attributes?

For btf_tag, only C language is supported, so duplicated function definition is not allowed. In general, btf_tag attributes will be accumulated during declaration, re-declaration and definition. And we only emit debuginfo at final definition site.

> Anyway, yeah, if all this works out without the replacement style API, I /think/ that's probably the better call. Might be worth including a test like that to show how it's expected to be handled.

Okay. will change from replacement API to direct parameter API.

For testing, I already had some tests for attribute accumulation in clang/test above, e.g.,

  struct __tag1 __tag2 t1;
  struct t1 {
    int a;
  };
  
   struct __tag1 t2;
   struct __tag2 t2 {
     int a;
   };
  
   ...

I will add a few negative test to show when the debuginfo is not generated. For example, attribute for forward declaration will result in debuginfo.

> (do these attributes need to work on declared-but-not-defined types? (eg: a type declared, and then a function that takes pointer-to that type, but the type is never defined in this translation unit) If so, could you include testing. for that situation?)

No. I will add a test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106615