[PATCH] D128113: WIP: Clang: fix AST representation of expanded template arguments.

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

In D128113#3777353 <https://reviews.llvm.org/D128113#3777353>, @mizvekov wrote:

> @davrec  @alexfh
>
> I finally managed to have a talk to @rsmith about this.
>
> He thinks that, even if we can't come up with a better solution in time, the benefits of this patch justify the costs observed, as those substitution nodes are pretty useless without a way to index the pack, and having a rich AST is one of Clang's main goals.

I support that - re-merge for now, and consider ways to reduce costs going forward.

I thought this through every which way and could not avoid @mizvekov's conclusion, that the only other way would be another Subst* type node, from which the pack indices could be inferred, but only via non-straightforward code requiring a bird's eye view of the AST.

Taking a step back and reconsidering, it's clear the original way - just storing the pack index - is very much preferable to all that complexity.  The benefits justify the costs for most users, and for clang developers who would be burdened by the appearance of an obscure new type class.  And as I said previously I think broad flags governing how much sugar/non-semantic info is added to the AST, i.e. letting users balance benefits vs. costs for themselves, is the better way to handle these concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128113