[clang] Issue #63106: [сlang] Representation of ellipsis in AST (PR #80976)

[Erich Keane via cfe-commits]

erichkeane wrote:

> > Hi @AaronBallman Can you pls explain me your previous response _"It would be better for us to associate the data with catch statements specifically because there's a lot fewer of those than there are variable declarations in general."_
> > I mean storing the ellipsis's location in catch stmt is not going to work, we need to add certain flags/function in VarDecl. Can you please review which I have raised currently one more time?
> 
> `CXXCatchStmt` currently stores three things: the location of the `catch` keyword, the `Stmt` for the handler, and the `VarDecl` for the exception declaration. We currently represent a catch-all by setting `VarDecl` to null. What @Endilll was pointing out in the original issue is that walking the AST for a `CXXCatchStmt` causes us to walk both of those child nodes, including the null `VarDecl` in the case of a catch-all.
> 
> I was suggesting that we should not walk a null node in that case (so modify the various AST walkers to check for a null pointer in that case and not walk down it), and instead we should have `CXXCatchStmt` retain a `SourceLocation` for the ellipsis in the case where the `VarDecl` is null. This way, people who need to know "is this catch statement a catch-all" can still find where to print diagnostics related to the `...`, but we don't end up walking over a null node.
> 
> You're approaching this from a different angle, where we have a non-null `VarDecl` that we use to represent the `...`. We could go this route and there's some appeal to doing so, but I think in that case, we'd want a concrete AST node to represent it. e.g., we'd want to add `class EllipsisDecl : public VarDecl {};` that could then be used by `CXXCatchStmt` and `FunctionDecl`, etc. However, this is a more involved approach and requires additional thought as to whether it's worth the extra effort.
> 
> CC @erichkeane for additional opinions.

I VERY much don't think the extra decl is worth the effort.  The Ellipsis Location should just be stored on the `CXXCatchStmt`, and we can leave the the empty `VarDecl` alone.  Avoiding traversal is a possibility, though not necessary to this approach.

https://github.com/llvm/llvm-project/pull/80976