[PATCH] D55239: [codeview] Emit typedefs of types derived from incomplete types

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

In D55239#1318690 <https://reviews.llvm.org/D55239#1318690>, @aganea wrote:

> I just wanted to point out that this overall process of dropping UDT records in the compiler is not the behavior that MSVC exhibits. S_UDTs are (should be) stripped by the linker, not by the compiler.


I think it's fine to drop them in the compiler, because we're relying on another TU to emit them.  So this saves the linker from doing some unnecessary merging in cases where we know there's no benefit to emitting them in the compiler.

In D55239#1318690 <https://reviews.llvm.org/D55239#1318690>, @aganea wrote:

> It's also the linker's job (in my sense) to remap forward references to full complete types (albeit if you can it locally in the compiler, it is probably better).


In an ideal world, probably.  But this is also going to be rather slow.  In Reid's example above, which I'll repeat here:

  struct Opaque;
  typedef Opaque *OpaqueHandle;
  OpaqueHandle gv;

we can't emit a full declaration for the `S_UDT` for `Opaque`, because we don't even have one.  So the linker still has some work to do here if we're trying to be perfect, because if a full declaration exists anywhere in the program, we would want it to point to it.  On the other hand, if it was `struct Opaque {};` then the compiler might as well do the remapping to save the linker some work.

But I think this patch isn't trying to address the first case.  We can evaluate it in a followup, but since it has the potential to slow down link times, we should be careful.

As an aside, we should also be careful about any remapping done in the linker, since it has the potential to interfere with ghash.  In this case we're fine since we only ghash types and not symbols, and `S_UDT` is a symbol.  But it's something to keep in mind.


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

https://reviews.llvm.org/D55239