[libcxx-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

[Matheus Izvekov via Phabricator via libcxx-commits]

mizvekov added a comment.

In D131858#3763892 <https://reviews.llvm.org/D131858#3763892>, @erichkeane wrote:

> In D131858#3763878 <https://reviews.llvm.org/D131858#3763878>, @mizvekov wrote:
>
>> In D131858#3763851 <https://reviews.llvm.org/D131858#3763851>, @erichkeane wrote:
>>
>>> Just did a quick scroll through this (as it is quite large!), but the general idea seems like a fine one to me.  I AM concerned about how it interacts with the deferred concepts instantiation that I've been working on (https://reviews.llvm.org/D126907), particularly around the MLTAL work.
>>
>> I think I did rebase it on top of that at one point, thought it was short lived as it was reverted if I am not mistaken.
>> But I can certainly do it again if you merge yours first, and I am available any time to help if it happens the other way around.
>>
>> But the gist of the change is simple, you will simply have to pass in the templated declaration for every level that you push into the MLTAL.
>> I think that should be the only change that affects you, besides possibly churn in AST tests if you plan to have any.
>
> Part of the problem is that the concepts instantiation needs to reform the MLTAL AFTER the fact using `Sema::getTemplateInstantiationArgs` and `addInstantiatedParametersToScope`.  BUT I don't se the corresponding changes on quick look here.
>
> As far as which goes first, I obviously have my preference, since I'm about 10 months into that patch now :/  I'm still working through a couple of issues though.

`getTemplateInstantiationArgs` is updated in this patch to build the MLTAL with the new information.

I don' have any changes to `addInstantiatedParametersToScope`, as that doesn't modify MLTAL, it just forwards it to SubstType.

No worries about who merges first, you are racing @rsmith here, you will win easily :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858