[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGef4635452f3a: [clang][dataflow] Add support for structured bindings of tuple-like types. (authored by ymandel). Repository: rG LLVM Github

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D139544#3984835 , @xazax.hun wrote: > This approach looks good to me. Some context: we kept the CFGs lightweight > because it looks like we did not need to do any extensions for the Clang > Static Analyzer. I'm glad the

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139544/new/ https://reviews.llvm.org/D139544 ___ cfe-commits mailing list

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 481677. ymandel added a comment. remove commented out code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139544/new/ https://reviews.llvm.org/D139544 Files: clang/lib/Analysis/FlowSensitive/Transfer.cpp

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. This approach looks good to me. Some context: we kept the CFGs lightweight because it looks like we did not need to do any extensions for the Clang Static Analyzer. I'm glad the dataflow framework can also work with the current representation. On the other hand, I

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D139544#3984532 , @isuckatcs wrote: >> Sorry, I should fix my response above: I *never* see the BindingDecls in the >> CFG, whether or not they are used. > > It was my fault, I was wrong with the wording. I meant the

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 481651. ymandel added a comment. Simplifies the implementation and removes mention of any potential CFG problem. This new version does the work eagerly by forcing a location for the synthesized variable. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added a comment. > Sorry, I should fix my response above: I *never* see the BindingDecls in the > CFG, whether or not they are used. It was my fault, I was wrong with the wording. I meant the `DeclRefExpr`s that refer to the `BindingDecl`s. You can access the `BindingDecl`s through

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. > Those BindingDecls never appear in the CFG unless one of the is used. Sorry, I should fix my response above: I *never* see the `BindingDecl`s in the CFG, whether or not they are used. Can you send me an example CFG that contains the BindingDecls? Repository: rG

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. > Those BindingDecls never appear in the CFG unless one of the is used. Indeed. That''s the issue (which this discussion has helped clarify) -- the binding decls are not in the CFG and I was expecting them to be. > By the time you see this node, the variable has

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-09 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added a comment. Those `BindingDecl`s never appear in the CFG unless one of the is used. Take a look at the other cases. 1.) Arrays void foo() { int arr[2]; auto [i0, i1] = arr; } [B1] 1: int arr[2]; 2: arr 3: [B1.2] (ImplicitCastExpr,

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D139544#3982821 , @isuckatcs wrote: >> the temporary's construction should appear before, but the binding decls, >> which use the synthetic variables, should appear after > > I'm confused a bit here. Right now the CFG looks

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added a comment. > the temporary's construction should appear before, but the binding decls, > which use the synthetic variables, should appear after I'm confused a bit here. Right now the CFG looks like this: ... Based on what you say I assume you want something to

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Domján -- thanks for the detailed explanations -- this has been really helpful. > After this the get<>() calls simply use this unnamed copy to initialize the > elements from first to last, so everything seems to proceed in order in the > CFG. Agreed -- it definitely

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added a comment. > I'm particularly interested in the case of tuple-like containers, which is a > little different (though I think the same point holds) `std::pair<>` is actually tuple-like if that's what you meant. Only tuple-like types have holding vars inside a

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Gabor -- An alternative solution to this problem, assuming the current structure of the CFG, is to add a map to the `Environment` of type `VarDecl` -> `BindingDecl` that registers `BindingDecl`s in need of initialization. Then, I could modify the `DeclStmt`

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thanks for clarifying the issues here. A few thoughts: 1. Thanks for clarifying about the blank lines. I was wondering how to read that and I think that fills in an important missing detail for me. 2. I'm particularly interested in the case of tuple-like containers,

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-08 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added a comment. > If CFG can be updated so that the > synthesized variables occur in the block *before* the DeclStmt containing the > DecompositionDecl, then we can drop the workaround that is included in this > patch. I'm not sure that this is possible. If you look at the AST of the

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

2022-12-07 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: xazax.hun. Herald added subscribers: martong, rnkovacs. Herald added a reviewer: NoQ. Herald added a project: All. ymandel requested review of this revision. Herald added a project: clang. This patch adds interpretation of binding