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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
