[PATCH] D94019: Switching Clang UniqueInternalLinkageNamesPass scheduling to using the LLVM one with newpm.

Mon Jan 4 19:20:16 PST 2021

aeubanks added a comment.

In D94019#2478423 <https://reviews.llvm.org/D94019#2478423>, @dblaikie wrote:

> In D94019#2478378 <https://reviews.llvm.org/D94019#2478378>, @aeubanks wrote:
>
>> Oh sorry, yeah this isn't NFC.
>>
>> But I still don't think this needs a new test. We're going from a custom Clang implementation of adding passes to something more generic that's also more tested within LLVM.
>> So IMO we just need an end to end test, which we already have in unique-internal-linkage-names.cpp. The details are tested on the LLVM side.
>
> Then could the end to end test be tightened up to demonstrate how this patch/the change in pass order has produced new/different/desired behavior?

It could be, but again I don't see the point. Rather than thinking about testing within individual changes, I think thinking about the overall state of testing regarding UniqueInternalLinkageNames after this change is better.
The only Clang test we need is to make sure `PTO.UniqueLinkageNames` is properly flipped, since that is the only thing Clang does regarding UniqueInternalLinkageNames . We have that Clang test in unique-internal-linkage-names.cpp (we could have a test that checks the PTO if it were serialized as you've said, but unique-internal-linkage-names.cpp is fine). It would fail if we didn't do `PTO.UniqueLinkageNames = CodeGenOpts.UniqueInternalLinkageNames;`.
Testing exactly what `PTO.UniqueLinkageNames` does is better done in LLVM. And that has already been done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94019