|
|
|
|
/* @generated */
|
|
|
|
|
digraph cfg {
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_1" [label="1: Start binary_conditional::binaryConditional\nFormals: \nLocals: x:binary_conditional::X 0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X 0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X a:binary_conditional::X \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_1" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_14" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_2" [label="2: Exit binary_conditional::binaryConditional \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_3" [label="3: Destruction(Scope) \n _=*&x:binary_conditional::X [line 23, column 1]\n n$1=_fun_binary_conditional::X::~X(&x:binary_conditional::X*) injected [line 23, column 1]\n _=*&a:binary_conditional::X [line 23, column 1]\n n$3=_fun_binary_conditional::X::~X(&a:binary_conditional::X*) injected [line 23, column 1]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_3" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_2" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_4" [label="4: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X); [line 22, column 9]\n n$8=_fun_binary_conditional::getX(&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X*) assign_last [line 22, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_4" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_6" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_5" [label="5: + \n " ]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_5" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_12" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_6" [label="6: Call _fun_binary_conditional::X::operator_bool \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X [line 22, column 9]\n n$10=_fun_binary_conditional::X::operator_bool(&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X&) [line 22, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_6" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_7" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_6" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_8" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_7" [label="7: Prune (true branch, boolean exp) \n PRUNE(n$10, true); [line 22, column 9]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_7" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_9" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_8" [label="8: Prune (false branch, boolean exp) \n PRUNE(!n$10, false); [line 22, column 9]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_8" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_10" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_9" [label="9: ConditionalStmt Branch \n n$11=_fun_binary_conditional::X::X(&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X*,&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X&) [line 22, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_9" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_5" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_10" [label="10: ConditionalStmt Branch \n n$12=_fun_binary_conditional::X::X(&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X*,&a:binary_conditional::X&) [line 22, column 19]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_10" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_5" ;
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_11" [label="11: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X); [line 22, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_11" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_4" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_12" [label="12: Destruction(temporaries cleanup) \n n$13=_fun_binary_conditional::X::X(&x:binary_conditional::X*,&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X&) [line 22, column 9]\n _=*&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X [line 22, column 19]\n n$17=_fun_binary_conditional::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X*) injected [line 22, column 19]\n _=*&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X [line 22, column 19]\n n$15=_fun_binary_conditional::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X*) injected [line 22, column 19]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_12" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_3" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_13" [label="13: DeclStmt \n VARIABLE_DECLARED(x:binary_conditional::X); [line 22, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_13" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_11" ;
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_14" [label="14: DeclStmt \n VARIABLE_DECLARED(a:binary_conditional::X); [line 21, column 3]\n n$18=_fun_binary_conditional::X::X(&a:binary_conditional::X*) [line 21, column 5]\n " shape="box"]
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
|
|
|
|
|
|
|
|
|
|
"binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_14" -> "binaryConditional#binary_conditional#15641211300815748363.a0f7e256e24b7117cb94c66e5aa27a30_13" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_1" [label="1: Start binary_conditional::conditional\nFormals: \nLocals: x:binary_conditional::X 0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X 0$?%__sil_tmpSIL_materialize_temp__n$7:binary_conditional::X 0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X 0$?%__sil_tmp_temp_marker_n$8:_Bool a:binary_conditional::X \n " color=yellow style=filled]
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_1" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_21" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_2" [label="2: Exit binary_conditional::conditional \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_3" [label="3: Destruction(Scope) \n _=*&x:binary_conditional::X [line 28, column 1]\n n$1=_fun_binary_conditional::X::~X(&x:binary_conditional::X*) injected [line 28, column 1]\n _=*&a:binary_conditional::X [line 28, column 1]\n n$3=_fun_binary_conditional::X::~X(&a:binary_conditional::X*) injected [line 28, column 1]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_3" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_2" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_4" [label="4: + \n " ]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_4" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_14" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_5" [label="5: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X); [line 27, column 9]\n n$10=_fun_binary_conditional::getX(&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X*) assign_last [line 27, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_5" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_6" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_6" [label="6: Call _fun_binary_conditional::X::operator_bool \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X [line 27, column 9]\n n$12=_fun_binary_conditional::X::operator_bool(&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X&) [line 27, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_6" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_7" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_6" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_8" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_7" [label="7: Prune (true branch, boolean exp) \n PRUNE(n$12, true); [line 27, column 9]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_7" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_9" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_8" [label="8: Prune (false branch, boolean exp) \n PRUNE(!n$12, false); [line 27, column 9]\n " shape="invhouse"]
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_8" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_11" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_9" [label="9: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$7:binary_conditional::X); [line 27, column 18]\n n$14=_fun_binary_conditional::getX(&0$?%__sil_tmpSIL_materialize_temp__n$7:binary_conditional::X*) assign_last [line 27, column 18]\n *&0$?%__sil_tmp_temp_marker_n$8:_Bool=1 [line 27, column 18]\n " shape="box"]
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_9" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_10" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_10" [label="10: ConditionalStmt Branch \n n$15=_fun_binary_conditional::X::X(&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X*,&0$?%__sil_tmpSIL_materialize_temp__n$7:binary_conditional::X&) [line 27, column 18]\n " shape="box"]
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_10" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_4" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_11" [label="11: ConditionalStmt Branch \n n$16=_fun_binary_conditional::X::X(&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X*,&a:binary_conditional::X&) [line 27, column 27]\n " shape="box"]
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_11" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_4" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_12" [label="12: DeclStmt \n VARIABLE_DECLARED(0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X); [line 27, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_12" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_5" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_13" [label="13: ExprWithCleanups \n VARIABLE_DECLARED(0$?%__sil_tmp_temp_marker_n$8:_Bool); [line 27, column 9]\n *&0$?%__sil_tmp_temp_marker_n$8:_Bool=0 [line 27, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
[clang] fix bad interaction between ConditionalOperator and initializers
Summary:
This is several inter-connected changes together to keep the tests
happy.
The ConditionalOperator `b?t:e` is translated by first creating a
placeholder variable to temporarily store the result of the evaluation
in each branch, then the real thing we want to assign to reads that
variable. But, there are situations where that changes the semantics of
the expression, namely when the value created is a struct on the stack
(eg, a C++ temporary). This is because in SIL we cannot assign the
*address* of a program variable, only its contents, so by the time we're
out of the conditional operator we cannot set the struct value
correctly anymore: we can only set its content, which we did, but that
results in a "shifted" struct value that is one dereference away from
where it should be.
So a batch of changes concern `conditionalOperator_trans`:
- instead of systematically creating a temporary for the conditional,
use the `trans_state.var_exp_typ` provided from above if available
when translating `ConditionalOperator`
- don't even set anything if that variable was already initialized by
merely translating the branch expression, eg when it's a constructor
- fix long-standing TODO to propagate these initialization facts
accurately for ConditionalOperator (used by `init_expr_trans` to also
figure out if it should insert a store to the variable being
initialised or not)
The rest of the changes adapt some relevant other constructs to deal
with conditionalOperator properly now that it can set the current
variable itself, instead of storing stuff inside a temp variable. This
change was a problem because some constructs, eg a variable declaration,
will insert nodes that set up the variable before calling its
initialization, and now the initialization happens *before* that setup,
in the translation of the inner conditional operator, which naturally
creates nodes above the current one.
- add a generic helper to force a sequential order between two
translation results, forcing node creation if necessary
- use that in `init_expr_trans` and `cxxNewExpr_trans`
- adjust many places where `var_exp_typ` was incorrectly not reset when translating sub-expressions
The sequentiality business creates more nodes when used, and the
conditionalOperator business uses fewer temporary variables, so the
frontend results change quite a bit.
Note that biabduction tests were invaluable in debugging this. There
could be other constructs to adjust similarly to cxxNewExpr that were
not covered by the tests though.
Added tests in pulse that exercises the previous bug.
Reviewed By: da319
Differential Revision: D24796282
fbshipit-source-id: 0790c8d17
4 years ago
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_13" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_12" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_14" [label="14: ExprWithCleanups \n n$17=_fun_binary_conditional::X::X(&x:binary_conditional::X*,&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X&) [line 27, column 9]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_14" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_17" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_14" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_18" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_15" [label="15: + \n " ]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_15" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_19" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_16" [label="16: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$7:binary_conditional::X [line 27, column 27]\n n$23=_fun_binary_conditional::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$7:binary_conditional::X*) injected [line 27, column 27]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_16" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_15" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_17" [label="17: Prune (true branch, if) \n n$24=*&0$?%__sil_tmp_temp_marker_n$8:_Bool [line 27, column 27]\n PRUNE(n$24, true); [line 27, column 27]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_17" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_16" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_18" [label="18: Prune (false branch, if) \n n$24=*&0$?%__sil_tmp_temp_marker_n$8:_Bool [line 27, column 27]\n PRUNE(!n$24, false); [line 27, column 27]\n " shape="invhouse"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_18" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_15" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_19" [label="19: Destruction(temporaries cleanup) \n _=*&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X [line 27, column 27]\n n$21=_fun_binary_conditional::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$6:binary_conditional::X*) injected [line 27, column 27]\n _=*&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X [line 27, column 27]\n n$19=_fun_binary_conditional::X::~X(&0$?%__sil_tmpSIL_materialize_temp__n$5:binary_conditional::X*) injected [line 27, column 27]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_19" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_3" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_20" [label="20: DeclStmt \n VARIABLE_DECLARED(x:binary_conditional::X); [line 27, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_20" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_13" ;
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_21" [label="21: DeclStmt \n VARIABLE_DECLARED(a:binary_conditional::X); [line 26, column 3]\n n$25=_fun_binary_conditional::X::X(&a:binary_conditional::X*) [line 26, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_21" -> "conditional#binary_conditional#4777209206611953450.41decaebdce6325bd31c1d47d4647c45_20" ;
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_1" [label="1: Start binary_conditional::getX\nFormals: __return_param:binary_conditional::X*\nLocals: x:binary_conditional::X \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_1" -> "getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_6" ;
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_2" [label="2: Exit binary_conditional::getX \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_3" [label="3: Return Stmt \n n$0=*&__return_param:binary_conditional::X* [line 16, column 3]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_3" -> "getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_4" ;
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_4" [label="4: Return Stmt \n n$1=_fun_binary_conditional::X::X(n$0:binary_conditional::X*,&x:binary_conditional::X&) [line 16, column 10]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_4" -> "getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_5" ;
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_5" [label="5: Return Stmt \n _=*&x:binary_conditional::X [line 16, column 10]\n n$3=_fun_binary_conditional::X::~X(&x:binary_conditional::X*) injected [line 16, column 10]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_5" -> "getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_2" ;
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_6" [label="6: DeclStmt \n VARIABLE_DECLARED(x:binary_conditional::X); [line 15, column 3]\n n$5=_fun_binary_conditional::X::X(&x:binary_conditional::X*) [line 15, column 5]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_6" -> "getX#binary_conditional(class binary_conditional::X)#7708042186122353096.8825a5a3afa327848f6dcf77ec0e3f60_3" ;
|
|
|
|
|
"operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_1" [label="1: Start binary_conditional::X::operator_bool\nFormals: this:binary_conditional::X*\nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_1" -> "operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_3" ;
|
|
|
|
|
"operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_2" [label="2: Exit binary_conditional::X::operator_bool \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_3" [label="3: Return Stmt \n *&return:_Bool=1 [line 11, column 21]\n " shape="box"]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_3" -> "operator_bool#X#binary_conditional#(663222161121279878).1074e20ff76c2575638dad4d7c1539a7_2" ;
|
|
|
|
|
"X#X#binary_conditional#{14983956545993097243|constexpr}.13a5ec9d8c7205dc22870cc8a7cef45e_1" [label="1: Start binary_conditional::X::X\nFormals: this:binary_conditional::X* __param_0:binary_conditional::X&\nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"X#X#binary_conditional#{14983956545993097243|constexpr}.13a5ec9d8c7205dc22870cc8a7cef45e_1" -> "X#X#binary_conditional#{14983956545993097243|constexpr}.13a5ec9d8c7205dc22870cc8a7cef45e_2" ;
|
|
|
|
|
"X#X#binary_conditional#{14983956545993097243|constexpr}.13a5ec9d8c7205dc22870cc8a7cef45e_2" [label="2: Exit binary_conditional::X::X \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"X#X#binary_conditional#{16256882625138630442|constexpr}.48629076530eab10af9149c015b04938_1" [label="1: Start binary_conditional::X::X\nFormals: this:binary_conditional::X*\nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"X#X#binary_conditional#{16256882625138630442|constexpr}.48629076530eab10af9149c015b04938_1" -> "X#X#binary_conditional#{16256882625138630442|constexpr}.48629076530eab10af9149c015b04938_2" ;
|
|
|
|
|
"X#X#binary_conditional#{16256882625138630442|constexpr}.48629076530eab10af9149c015b04938_2" [label="2: Exit binary_conditional::X::X \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"X#X#binary_conditional#{9078254210141761193|constexpr}.75e8dfb872b829fb1a0edca3193e1301_1" [label="1: Start binary_conditional::X::X\nFormals: this:binary_conditional::X* __param_0:binary_conditional::X const &\nLocals: \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
"X#X#binary_conditional#{9078254210141761193|constexpr}.75e8dfb872b829fb1a0edca3193e1301_1" -> "X#X#binary_conditional#{9078254210141761193|constexpr}.75e8dfb872b829fb1a0edca3193e1301_2" ;
|
|
|
|
|
"X#X#binary_conditional#{9078254210141761193|constexpr}.75e8dfb872b829fb1a0edca3193e1301_2" [label="2: Exit binary_conditional::X::X \n " color=yellow style=filled]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
}
|
