rsmith added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplate.cpp:2174 + Args.addOuterTemplateArguments(SubstArgs); + Args.addOuterRetainedLevel(); + NamedDecl *NewParam = transformTemplateParameter(Param, Args); ---------------- aeubanks wrote: > rsmith wrote: > > This outer retained level would correspond to the parameters you're > > building -- we shouldn't add this. Instead, we should have two different > > `SubstArgs` lists, one for the outer parameters and one for the inner > > parameters; we should add just the outer parameters here, and add the inner > > and outer parameters below. (And at the end of each iteration of this loop > > we should accumulate arguments onto the outer arguments list like we > > accumulate arguments onto the inner arguments list in the next loop.) > I think this is what you're referring to? I still don't understand exactly > what `Args.addOuterRetainedLevel()` does, and the current draft doesn't fix > the modules crash. I found a few more places that need to be adjusted. Here's where I got to: https://github.com/zygoloid/llvm-project/commit/fffb4b688eb5a91bc2aca673773bc2da018f3dba (this is on top of the earlier version of this patch, though). There are still some test failures there, so I think some more updates are still needed. We also need to do the same cloning process in `buildSimpleDeductionGuide`, which I think is why the modules unit test is still failing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116983/new/ https://reviews.llvm.org/D116983 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
