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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 4 07:02:15 PST 2024


AaronBallman 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.

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


More information about the cfe-commits mailing list