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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
