[PATCH] D20435: [codeview] Adding support for CodeView types

[Reid Kleckner via llvm-commits]

rnk added a comment.

In http://reviews.llvm.org/D20435#436415, @aaboud wrote:

> Hi all,
>  Thanks a lot for all your valuable comments.
>
> Reid, I agree with you that we can reuse TypeRecord instead of introducing a new CodeViewType.
>  I also understand your design and approach. And I am willing to refactor my patch to suit them.


Great!

> However, I must mention two concerns:

> 

> 1. Can we be sure that we can generate the correct index for each type record during creation of that record?

>   - In some cases you want to revisit the type record you generated to fix some of its attributes (like the attribute "ClassOptions::ContainsNestedClass").


I guess the issue is that classes don't contain their nested classes, instead the nested class points back at the parent record through its scope field. I think here we've found the first point where we'll need to extend the existing metadata. Consider the IR metadata generated for this example:

  struct A {
    int a;
    struct B { int b; };
  };
  A a;
  //A::B b;

Today the metadata we generate does not mention B at all, and that is a problem we're going to have to fix if we want our type records to be bitwise identical across TUs. I think we should change clang to mention a forward declaration of B in the elements list of A, and then we don't need to go back and patch up existing type records.

> 2. PDB support

>   - I know that you do not want to support PDB, but we must keep it into consideration.

>   - When emitting types to PDB we do not know the index of each type record before we actually emit the record.

>   - In the current design we create the type record index when we write the record (before we emit it to the object/pdb file).


I assume the way we support types in PDBs is by doing synchronous IPCs or file IO at the point of the call to `TypeTableBuilder::writeRecord`. Is that not similar to what MSVC does?

> Once again, the approach you have in mind can work if we solve the above two issues.

>  One way to solve them is to allow fixing data in the written record, although it would be better if we can delay the creation of the record buffer till we know the final value of the data.

> 

> Any thoughts?



http://reviews.llvm.org/D20435