[PATCH] D26840: [ThinLTO] Fix crash when importing an opaque type

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D26840#599898, @mehdi_amini wrote:

> >> But the block of code you modified is only entered if we identify the dest type as a struct type:
> >> 
> >>   if (cast<StructType>(DstTy)->isOpaque()) {
> > 
> > So at that point we are detecting the dest opaque type as a StructType, so why wouldn't we catch those here too?
>
> Sorry I was wrong in my first walkthrough, the issue is that we initialize the type finder with:
>
> `StructTypes.run(M, true);`
>
> The "true" means "OnlyNamed", this is why we don't have it. I have no idea why it only gets the named one here or what would be the impact to get them all.


Tracked it down to this commit from 2012:

--------------------

commit d46575f1908b1fb9950e610a1f36733893ad44b1
Author: Bill Wendling <isanbard at gmail.com>
Date:   Sat Apr 21 23:59:16 2012 +0000

  Add a flag to the struct type finder to collect only those types which have
  names. This saves collecting types we normally don't care about.
  
  
  git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@155300 91177308-0d34-0410-b5e6-96231b3b80d8

----------------------

Looks like it was an optimization. I think the right place to fix it is here - just collect all struct types. The current patch seems like a bandaid for the fact that we aren't doing this up front here.


https://reviews.llvm.org/D26840