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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D139544#3982821 <https://reviews.llvm.org/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 like this:
>
>   <constructor_call>
>   <binding_decl_0>
>   ...
>   <binding_decl_n>
>
> Based on what you say I assume you want something to happen between `constructor_call` and `binding_decl_0`. 
> Could you visualize somehow how you think the ideal CFG here looks like?

Sorry, I should have explained in more detail (refernce example: https://godbolt.org/z/dE9ojcjj7). *Perhaps I'm still misreading the CFG, but I don't see the binding decls serialized in the CFG, only the synthesized var decls.* That is, here's my reading of the CFG:
The resulting simplified CFG would be:

  <constructor_call>
  <synthesized_var_decl_0>
  ...
  <synthesized_var_decl_n>

In more detail, here is my annotated reading of the CFG, based on our discussion:

  [B1]
     1: mk
     2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, std::tuple<int, int> (*)(void))
     3: [B1.2]() (CXXRecordTypedCall, [B1.4])
     4: auto = mk();  // <constructor_call> (with <binding_decl_0>, <binding_decl_1> nested as children)
     5: get<0UL>
     6: [B1.5] (ImplicitCastExpr, FunctionToPointerDecay, typename tuple_element<0, tuple<int, int> >::type (*)(tuple<int, int>))
     7:  // <ref to anonymous temporary defined on line 4>
     8: [B1.7] (ImplicitCastExpr, NoOp, std::tuple<int, int>)
     9: [B1.8] (CXXConstructExpr, [B1.10]+0, tuple<int, int>)
    10: [B1.6]([B1.9])
    11: [B1.10]
    12: std::tuple_element<0, std::tuple<int, int>>::type i0 = get<0UL>(); // <synth_var_decl_0>
    13: get<1UL>
    14: [B1.13] (ImplicitCastExpr, FunctionToPointerDecay, typename tuple_element<1, tuple<int, int> >::type (*)(tuple<int, int>))
    15: // <ref to anonymous temporary defined on line 4>
    16: [B1.15] (ImplicitCastExpr, NoOp, std::tuple<int, int>)
    17: [B1.16] (CXXConstructExpr, [B1.18]+0, tuple<int, int>)
    18: [B1.14]([B1.17])
    19: [B1.18]
    20: std::tuple_element<1, std::tuple<int, int>>::type i1 = get<1UL>(); // <synth_var_decl_1>
    21: i0 // ref to <binding_decl_0>
    22: (void)[B1.21] (CStyleCastExpr, ToVoid, void)

Here is the crux of the issue: per the AST, line 21 is reference is to the *`BindingDecl`*, not the (synthesized) `VarDecl`:

  -DeclRefExpr <col:9> 'std::tuple_element<0, std::tuple<int, int>>::type':'int' lvalue Binding 0x559942acf350 'i0' 'std::tuple_element<0, std::tuple<int, int>>::type':'int'

So, in order to make sense of that reference, I need to define the `BindingDecl`. But, that in turn is defined in terms of the (synthesized) `VarDecl`. So, I want to insert between elements 20 and 21:

  20.a: <binding_decl_0> // references synthesized variable `i0` from line 12 in its binding
  20.b: <binding_decl_1> // references synthesized variable `i1` from line 20 in its binding

The resulting simplified CFG would be:

  <constructor_call>
  <synthesized_var_decl_0>
  ...
  <synthesized_var_decl_n>
  <binding_decl_0>
  ...
  <binding_decl_n>

I suggested repeating the `DecompositionDecl` because it includes the `BindingDecls`, but spelling them out explicitly would probably be even better.  Right now, the only chance I get to visit the `BindingDecls` is when I encounter line 4, as children of the `DecompositionDecl`, at which point the `binding` of each decl reference as-yet-undefined (synthesized) `VarDecl`s, hence my dilemma.

I'm fine with a custom solution -- that's indeed what this patch contains and I'm happy to leave it at that.

Thank you, again, for your patience hashing this out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139544