sgatev marked 2 inline comments as done.
sgatev added inline comments.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:80
+///
+/// FIXME: Consider adding support for structured bindings to the CFG builder.
+class DecompositionVisitor : public ConstStmtVisitor<DecompositionVisitor> {
----------------
xazax.hun wrote:
> sgatev wrote:
> > xazax.hun wrote:
> > > Did you look into how hard would it be to add structured bindings to the
> > > CFG builder? If the effort is comparable to this patch (and not
> > > significantly bigger), it might be better to do that work instead of
> > > spending effort on some temporary workaround. What do you think?
> > Circling back to this after a while. I believe we explored changing the CFG
> > briefly, but don't have a fully fleshed out proposal for it. I recently
> > noticed
> > https://discourse.llvm.org/t/implement-support-for-c-17-structured-bindings-in-the-clang-static-analyzer/60588.
> > It seems that part of the project is exploring necessary changes to the
> > CFG. What do you think about submitting this patch with local pattern
> > matching and revisiting that once the GSoC project completes?
> Officially, there is no candidate accepted for that project yet, so there is
> no guarantee that we will have someone working on this. That being said, the
> CSA devs started to discuss what would be the best way to represent
> structured bindings in the CFG, and it is possible that for many of the cases
> we do not actually need to change the CFG, because the AST nodes have rich
> information. But of course, none of this is final, but we do not need to
> block this patch on that. So I'm fine adding this for now.
>
> The AST for the supported case looks like:
> ```
> `-DeclStmt 0x5599ad9de6b0 <line:8:7, col:45>
> `-DecompositionDecl 0x5599ad9de310 <col:7, col:42> col:13 used 'A &'
> cinit
> |-DeclRefExpr 0x5599ad9de388 <col:42> 'A' lvalue Var 0x5599ad9ddb80
> 'Baz' 'A'
> |-BindingDecl 0x5599ad9de280 <col:14> col:14 BoundFooRef 'int'
> | `-MemberExpr 0x5599ad9de630 <col:14> 'int' lvalue .Foo
> 0x5599ad9dd970
> | `-DeclRefExpr 0x5599ad9de610 <col:14> 'A':'A' lvalue
> Decomposition 0x5599ad9de310 '' 'A &'
> `-BindingDecl 0x5599ad9de2c8 <col:27> col:27 BoundBarRef 'int'
> `-MemberExpr 0x5599ad9de680 <col:27> 'int' lvalue .Bar
> 0x5599ad9dd9d8
> `-DeclRefExpr 0x5599ad9de660 <col:27> 'A':'A' lvalue
> Decomposition 0x5599ad9de310 '' 'A &'
> ```
>
> So each `BindingDecl` is like:
> ```
> `-BindingDecl 0x5599ad9de2c8 <col:27> col:27 BoundBarRef 'int'
> `-MemberExpr 0x5599ad9de680 <col:27> 'int' lvalue .Bar
> 0x5599ad9dd9d8
> `-DeclRefExpr 0x5599ad9de660 <col:27> 'A':'A' lvalue
> Decomposition 0x5599ad9de310 '' 'A &'
> ```
>
> Is there a case where the `BindingDecl` would have a different shape? If we
> do not expect that happening, I feel like an `StmtVisitor` is a bit of an
> overkill for this and we could handle the supported case more directly. What
> do you think?
Agreed. I simplified the patch. Please take a look.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120495/new/
https://reviews.llvm.org/D120495
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
