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