[PATCH] D41669: Teach ValueMapper to use ODR uniqued types when available

Mon Feb 15 16:41:29 PST 2021

tejohnson added a comment.

In D41669#2564314 <https://reviews.llvm.org/D41669#2564314>, @dexonsmith wrote:

> In D41669#2564247 <https://reviews.llvm.org/D41669#2564247>, @tejohnson wrote:
>
>> Sorry to be unclear. That test case, added with this patch, does still fail for me with this change reverted. It will fail if there is more than one DICompositeType - please take a look to see if this looks problematic. I don't recall exactly why I was concerned about that originally. What doesn't fail for me anymore with this change reverted is the test case in the associated PR, which was an actual crash.
>>
>> Note all of this was before you pushed 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a <https://reviews.llvm.org/rG22a52dfddcefad4f275eb8ad1cc0e200074c2d8a> so didn't have that.
>
> Aha, I get it; I was reading too fast before.
>
> I think the test you added is a good one; better not to clone the composite type unnecessarily. I posted the "revert" at https://reviews.llvm.org/D96734, and it updates CloneFunctionInto to keep the testcase passing.

Great, thanks for fixing this!

Repository:
  rL LLVM

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

https://reviews.llvm.org/D41669