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

[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 `DecompositionDecl`.

> Here, both lines 7 and 13 are blank but seem to be the underlying tuple that's being deconstructed. Are these different instances of the DecompositionDecl or are they (unprinted) references back to line 4?

Those lines are what you see as `DeclRefExpr Decomposition ''` in the AST. Which are indeed references back to line 4 as you mentioned.

> I'm still bothered that this seems to violate the design/philosophy of the CFG, which I take as ensuring that elements proceed in the order of operation.

I think in this case the elements do proceed in order of operation. When you create a structured binding by value to a tuple like type, the copy constructor of the type is invoked first.

Let's assume that you have a tuple-like `std::pair<int, int> p{1, 2};` and you create the structured binding `auto [a, b] = p;`.

1. The copy constructor of `p` is invoked first and a copy get's created. Let's call this copy `tmp`. Note that if the copy constructor has side effects, they will also be visible.
2. A new variable for each element in the tuple is created and is initialized by `get<idx>(tmp);`. It is important that the initialization happens from the copy, `tmp` and not from the original tuple `p`.

The CFG resembles this behaviour. If you look at both of the example CFGs in this discussion, you can see that both of them begin with the constructor invocation.

   7: p
   8: [B1.7] (ImplicitCastExpr, NoOp, const pair<int, int>)
   9: [B1.8] (CXXConstructExpr, [B1.10], std::pair<int, int>)
  10: auto = p;

  1: mk
  2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, std::tuple<_Bool, int> (*)(void))
  3: [B1.2]() (CXXRecordTypedCall, [B1.4])
  4: auto = mk();

Points `4` and `10` are a bit weird, but that's where the copy is created, however the copy is unnamed. This unnamed copy is what I refered to as `tmp`.

Note that when you do a regular copy-constructor invocation like

  std::tuple<bool, int> mk;
  std::tuple<bool, int> mk2 = mk;

you would get

  3: mk
  4: [B1.3] (ImplicitCastExpr, NoOp, const class std::tuple<_Bool, int>)
  5: [B1.4] (CXXConstructExpr, [B1.6], std::tuple<_Bool, int>)
  6: std::tuple<bool, int> mk2 = mk;
                           ^^^This is the name that's missing from the CFGs above.

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.

> Finally, a nit: why doesn't line 13 in your example, and lines 7 and 13 in my example, print? Is that something that I could add to the CFG printer?

If I remember correctly (and my intuition is correct based on the examples) a `DeclRefExpr` in the CFG is the name of the variable being referenced. 
So for example `DeclRefExpr 'p'`  is simply `p` in the CFG.

If you look at the AST in my example, you can see that the `DeclRefExpr` to the decomposition is `DeclRefExpr Decomposition ''`, where  `''` 
is supposed to be the name of the variable. So I think the empty line comes from here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139544/new/

https://reviews.llvm.org/D139544