From 4c4168a726c05eaf6f4edc4832628c6cf8edf2e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Villemot?= <sebastien@dynare.org>
Date: Wed, 8 Jul 2020 12:13:35 +0200
Subject: [PATCH] Unary ops substitution: bugfix, actually restrict the
substitution to the chosen equations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
By default, the preprocessor is supposed to only do the “unary ops”
transformation in the equations of VAR/PAC/trend component models.
However, the implementation was slightly different so far. It would detect
candidates to this transformation in the chosen equations, but it would then
perform the substitution in *all* equations.
This could lead for crashes, for example if the chosen equation contains
log(X(-1)), but another (non-chosen) equation has log(X(-2)). Then this latter
expression, even though it belongs to the same lag-equivalence class, is not
properly handled, causing a segfault.
Also do a few related cosmetic changes.
(cherry picked from commit 5b80a4db590a68cba59529f6b46e101ad355b04e)
---
src/DynamicModel.cc | 15 +++++++--------
src/ExprNode.cc | 4 +++-
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/DynamicModel.cc b/src/DynamicModel.cc
index 453aa9c5..57d059eb 100644
--- a/src/DynamicModel.cc
+++ b/src/DynamicModel.cc
@@ -6313,9 +6313,8 @@ DynamicModel::substituteUnaryOps(const vector<int> &eqnumbers)
set<int> used_local_vars;
for (int eqnumber : eqnumbers)
equations[eqnumber]->collectVariables(SymbolType::modelLocalVariable, used_local_vars);
- for (auto &it : local_variables_table)
- if (used_local_vars.find(it.first) != used_local_vars.end())
- it.second->findUnaryOpNodesForAuxVarCreation(nodes);
+ for (int mlv : used_local_vars)
+ local_variables_table[mlv]->findUnaryOpNodesForAuxVarCreation(nodes);
// Mark unary ops to be substituted in selected equations
for (int eqnumber : eqnumbers)
@@ -6323,16 +6322,16 @@ DynamicModel::substituteUnaryOps(const vector<int> &eqnumbers)
// Substitute in model local variables
vector<BinaryOpNode *> neweqs;
- for (auto &it : local_variables_table)
- it.second = it.second->substituteUnaryOpNodes(nodes, subst_table, neweqs);
+ for (int mlv : used_local_vars)
+ local_variables_table[mlv] = local_variables_table[mlv]->substituteUnaryOpNodes(nodes, subst_table, neweqs);
// Substitute in equations
- for (auto &equation : equations)
+ for (int eq : eqnumbers)
{
- auto substeq = dynamic_cast<BinaryOpNode *>(equation->
+ auto substeq = dynamic_cast<BinaryOpNode *>(equations[eq]->
substituteUnaryOpNodes(nodes, subst_table, neweqs));
assert(substeq);
- equation = substeq;
+ equations[eq] = substeq;
}
// Add new equations
diff --git a/src/ExprNode.cc b/src/ExprNode.cc
index 218f0793..ca91b743 100644
--- a/src/ExprNode.cc
+++ b/src/ExprNode.cc
@@ -3614,7 +3614,9 @@ UnaryOpNode::substituteUnaryOpNodes(const lag_equivalence_table_t &nodes, subst_
else
subst_table[rit->second] = dynamic_cast<VariableNode *>(aux_var->decreaseLeadsLags(base_index - rit->first));
- return const_cast<VariableNode *>(subst_table.find(this)->second);
+ assert(subst_table.find(this) != subst_table.end());
+
+ return const_cast<VariableNode *>(subst_table.at(this));
}
expr_t
--
GitLab