mizvekov added a comment.

In D128113#3745888 <https://reviews.llvm.org/D128113#3745888>, @alexfh wrote:

> The main questions we need to answer here are:
>
> 1. is the cost of storing parameter pack substitution indices while building 
> AST worth the benefits?
> 2. is there an alternative solution that would shift the cost to the moment 
> when this information is about to be used (when formatting diagnostic, I 
> suppose)?
> 3. is there a good way to make this cost optional (using a flag or somehow 
> else)?



1. For me yes, this was a fairly simple patch that got reviewed quickly and got 
the job done. But it seems it was not good for your use case :)
2. I think there is a chance the approach I mentioned earlier could work. It's 
hard for me to tell without spending some time trying. This could though end up 
being a much more complicated patch, which hits my projects bottleneck: it's 
pretty hard to find reviewers for this sort of stuff.
3. That has the advantage of change simplicity, but not user simplicity. It 
could provide a escape hatch in allowing us to have a simple solution now, but 
then iterate to something better later. But since this is an externally visible 
change to the AST, it does seem far from ideal to be changing this twice. It 
would be pretty unfortunate if someone else started using this feature and then 
we decided to take it away.

I would be fine though reverting this, and then doing some more investigation, 
and also perhaps waiting for a second opinion from @rsmith.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128113

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to