Skip to content

Commit

Permalink
Check for use before assign in functions (#9761)
Browse files Browse the repository at this point in the history
Fixes #9717
  • Loading branch information
perost committed Nov 25, 2022
1 parent 6742f22 commit 0f49428
Show file tree
Hide file tree
Showing 35 changed files with 694 additions and 27 deletions.
198 changes: 198 additions & 0 deletions OMCompiler/Compiler/NFFrontEnd/NFFunction.mo
Expand Up @@ -1451,6 +1451,10 @@ uniontype Function
end if;
end if;
if not InstContext.inRelaxed(context) then
checkUseBeforeAssign(fn);
end if;
// Sort the local variables based on their dependencies.
fn.locals := sortLocals(fn.locals, InstNode.info(fn.node));
end typeFunctionBody;
Expand Down Expand Up @@ -2599,6 +2603,200 @@ protected
UnorderedMap.add(key, true, interface_map);
end for;
end getDerivative;
function checkUseBeforeAssign
"Checks if any output or local variables in a function are used
uninitialized and prints warnings for such uses."
input Function fn;
protected
Vector<InstNode> unassigned;
list<Statement> body;
algorithm
// Skip external and builtin functions.
if isExternal(fn) or isBuiltin(fn) then
return;
end if;
unassigned := Vector.new<InstNode>();
addUnassignedComponents(unassigned, fn.outputs);
addUnassignedComponents(unassigned, fn.locals);
body := getBody(fn);
checkUseBeforeAssign2(unassigned, body);
end checkUseBeforeAssign;
function addUnassignedComponents
input Vector<InstNode> unassigned;
input list<InstNode> variables;
protected
Type ty;
algorithm
for var in variables loop
ty := InstNode.getType(var);
if Type.isScalarBuiltin(ty) and not Component.hasBinding(InstNode.component(var)) then
Vector.push(unassigned, var);
end if;
end for;
end addUnassignedComponents;
function checkUseBeforeAssign2
input Vector<InstNode> unassigned;
input list<Statement> statements;
protected
SourceInfo info;
algorithm
for stmt in statements loop
info := Statement.info(stmt);
() := match stmt
case Statement.ASSIGNMENT()
algorithm
checkUseBeforeAssignExp(unassigned, stmt.rhs, info);
markAssignedOutput(unassigned, stmt.lhs);
then
();
case Statement.FOR()
algorithm
if isSome(stmt.range) then
checkUseBeforeAssignExp(unassigned, Util.getOption(stmt.range), info);
end if;
checkUseBeforeAssign2(unassigned, stmt.body);
then
();
case Statement.IF()
algorithm
checkUseBeforeAssignIf(unassigned, stmt.branches, info);
then
();
case Statement.ASSERT()
algorithm
checkUseBeforeAssignExp(unassigned, stmt.condition, info);
checkUseBeforeAssignExp(unassigned, stmt.message, info);
checkUseBeforeAssignExp(unassigned, stmt.level, info);
then
();
case Statement.WHILE()
algorithm
checkUseBeforeAssignExp(unassigned, stmt.condition, info);
checkUseBeforeAssign2(unassigned, stmt.body);
then
();
else ();
end match;
end for;
end checkUseBeforeAssign2;
function markAssignedOutput
input Vector<InstNode> unassigned;
input Expression assignedExp;
protected
InstNode node;
Integer index;
algorithm
() := match assignedExp
case Expression.CREF()
algorithm
node := ComponentRef.node(ComponentRef.last(assignedExp.cref));
(_, index) := Vector.find(unassigned, function InstNode.refEqual(node1 = node));
if index > 0 then
Vector.remove(unassigned, index);
end if;
then
();
case Expression.TUPLE()
algorithm
for e in assignedExp.elements loop
markAssignedOutput(unassigned, e);
end for;
then
();
else ();
end match;
end markAssignedOutput;
function checkUseBeforeAssignIf
input Vector<InstNode> unassigned;
input list<tuple<Expression, list<Statement>>> branches;
input SourceInfo info;
protected
Vector<InstNode> unassigned_branch;
list<InstNode> assigned = {};
Integer index;
algorithm
for b in branches loop
checkUseBeforeAssignExp(unassigned, Util.tuple21(b), info);
end for;
// Check each branch separately, then assume that any variables that were
// assigned (or incorrectly used) in at least one branch are assigned after
// the if-statement to avoid false positives.
for b in branches loop
unassigned_branch := Vector.copy(unassigned);
checkUseBeforeAssign2(unassigned_branch, Util.tuple22(b));
if Vector.size(unassigned) <> Vector.size(unassigned_branch) then
assigned := listAppend(
List.setDifferenceOnTrue(Vector.toList(unassigned), Vector.toList(unassigned_branch), InstNode.refEqual),
assigned);
end if;
end for;
if not listEmpty(assigned) then
assigned := List.uniqueOnTrue(assigned, InstNode.refEqual);
for a in assigned loop
(_, index) := Vector.find(unassigned, function InstNode.refEqual(node1 = a));
if index > 0 then
Vector.remove(unassigned, index);
end if;
end for;
end if;
end checkUseBeforeAssignIf;
function checkUseBeforeAssignExp
input Vector<InstNode> unassigned;
input Expression exp;
input SourceInfo info;
algorithm
Expression.apply(exp,
function checkUseBeforeAssignExp_traverse(unassigned = unassigned, info = info));
end checkUseBeforeAssignExp;
function checkUseBeforeAssignExp_traverse
input Vector<InstNode> unassigned;
input Expression exp;
input SourceInfo info;
protected
Integer index;
InstNode node;
algorithm
() := match exp
case Expression.CREF()
algorithm
node := ComponentRef.node(ComponentRef.last(exp.cref));
(_, index) := Vector.find(unassigned, function InstNode.refEqual(node1 = node));
if index > 0 then
Vector.remove(unassigned, index);
Error.addSourceMessage(Error.WARNING_DEF_USE, {InstNode.name(node)}, info);
end if;
then
();
else ();
end match;
end checkUseBeforeAssignExp_traverse;
end Function;
annotation(__OpenModelica_Interface="frontend");
Expand Down
37 changes: 37 additions & 0 deletions testsuite/flattening/modelica/scodeinst/FunctionUnitialized1.mo
@@ -0,0 +1,37 @@
// name: FunctionUnitialized1
// keywords:
// status: correct
// cflags: -d=newInst
//
//

model FunctionUnitialized1
Real y = f(time);
function f
input Real x;
output Real y;
protected
Real z;
algorithm
y := z + x + y;
y := y + 1;
end f;
end FunctionUnitialized1;

// Result:
// function FunctionUnitialized1.f
// input Real x;
// output Real y;
// protected Real z;
// algorithm
// y := z + x + y;
// y := y + 1.0;
// end FunctionUnitialized1.f;
//
// class FunctionUnitialized1
// Real y = FunctionUnitialized1.f(time);
// end FunctionUnitialized1;
// [flattening/modelica/scodeinst/FunctionUnitialized1.mo:16:6-16:20:writable] Warning: z was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [flattening/modelica/scodeinst/FunctionUnitialized1.mo:16:6-16:20:writable] Warning: y was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
//
// endResult
48 changes: 48 additions & 0 deletions testsuite/flattening/modelica/scodeinst/FunctionUnitialized2.mo
@@ -0,0 +1,48 @@
// name: FunctionUnitialized2
// keywords:
// status: correct
// cflags: -d=newInst
//
//

model FunctionUnitialized2
Real y = f(time);

function f
input Real x;
output Real y;
protected
Real z, w, u;
algorithm
if x > 1 then
z := 0;
else
u := 1;
end if;

y := z + w + u;
end f;
end FunctionUnitialized2;

// Result:
// function FunctionUnitialized2.f
// input Real x;
// output Real y;
// protected Real z;
// protected Real w;
// protected Real u;
// algorithm
// if x > 1.0 then
// z := 0.0;
// else
// u := 1.0;
// end if;
// y := z + w + u;
// end FunctionUnitialized2.f;
//
// class FunctionUnitialized2
// Real y = FunctionUnitialized2.f(time);
// end FunctionUnitialized2;
// [flattening/modelica/scodeinst/FunctionUnitialized2.mo:23:5-23:19:writable] Warning: w was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
//
// endResult
2 changes: 2 additions & 0 deletions testsuite/flattening/modelica/scodeinst/Makefile
Expand Up @@ -662,6 +662,8 @@ FunctionSections3.mo \
FunctionSections4.mo \
FunctionSections5.mo \
FunctionStreamPrefix.mo \
FunctionUnitialized1.mo \
FunctionUnitialized2.mo \
IfConnect1.mo \
IfConnect2.mo \
IfConnect3.mo \
Expand Down
7 changes: 7 additions & 0 deletions testsuite/openmodelica/dataReconciliation/TSP_FourFlows.mos
Expand Up @@ -2270,5 +2270,12 @@ getErrorString();
// [ThermoSysPro 3.2.0/WaterSteam/Junctions/Mixer2.mo:17:3-18:52:writable] Warning: Connector Cs is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [ThermoSysPro 3.2.0/WaterSteam/Junctions/Mixer2.mo:20:3-22:17:writable] Warning: Connector Ce1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SourceQ.mo:24:3-25:52:writable] Warning: Connector C is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:784:9-784:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:851:9-851:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:1089:9-1089:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph.mo:76:3-76:60:writable] Warning: dh1satp was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph.mo:76:3-76:60:writable] Warning: dh2satp was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph_der.mo:179:3-182:49:writable] Warning: du1satp_der was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph_der.mo:179:3-182:49:writable] Warning: du2satp_der was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// "
// endResult
7 changes: 7 additions & 0 deletions testsuite/openmodelica/dataReconciliation/TSP_FourFlows1.mos
Expand Up @@ -2484,5 +2484,12 @@ getErrorString();
// [ThermoSysPro 3.2.0/WaterSteam/Junctions/Mixer2.mo:20:3-22:17:writable] Warning: Connector Ce1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SourceP.mo:30:3-31:45:writable] Warning: Connector C is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/Sink.mo:17:3-19:16:writable] Warning: Connector C is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:784:9-784:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:851:9-851:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:1089:9-1089:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph.mo:76:3-76:60:writable] Warning: dh1satp was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph.mo:76:3-76:60:writable] Warning: dh2satp was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph_der.mo:179:3-182:49:writable] Warning: du1satp_der was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph_der.mo:179:3-182:49:writable] Warning: du2satp_der was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// "
// endResult
27 changes: 26 additions & 1 deletion testsuite/openmodelica/dataReconciliation/TSP_FourFlows10.mos
Expand Up @@ -3192,5 +3192,30 @@ getErrorString();
// stdout | info | DataReconciliation Completed!
// "
// end SimulationResult;
// ""
// "[openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/Sink.mo:17:3-19:16:writable] Warning: Connector C is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:20:3-22:16:writable] Warning: Connector C1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:23:3-24:52:writable] Warning: Connector C2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:20:3-22:16:writable] Warning: Connector C1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:23:3-24:52:writable] Warning: Connector C2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:20:3-22:16:writable] Warning: Connector C1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:23:3-24:52:writable] Warning: Connector C2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:20:3-22:16:writable] Warning: Connector C1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SingularPressureLoss.mo:23:3-24:52:writable] Warning: Connector C2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:20:3-22:42:writable] Warning: Connector Ce1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:23:3-25:37:writable] Warning: Connector Ce2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:26:3-28:37:writable] Warning: Connector Cs1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:29:3-31:43:writable] Warning: Connector Cs2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:20:3-22:42:writable] Warning: Connector Ce1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:23:3-25:37:writable] Warning: Connector Ce2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:26:3-28:37:writable] Warning: Connector Cs1 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/VolumeATh.mo:29:3-31:43:writable] Warning: Connector Cs2 is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [openmodelica/dataReconciliation/NewDataReconciliationSimpleTests/SourcePQ.mo:29:3-30:52:writable] Warning: Connector C is not balanced: The number of potential variables (4) is not equal to the number of flow variables (0).
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:784:9-784:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:851:9-851:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteam/IF97_packages.mo:1089:9-1089:27:writable] Warning: cv was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph.mo:76:3-76:60:writable] Warning: dh1satp was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph.mo:76:3-76:60:writable] Warning: dh2satp was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph_der.mo:179:3-182:49:writable] Warning: du1satp_der was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// [ThermoSysPro 3.2.0/Properties/WaterSteamSimple/prop4_Ph_der.mo:179:3-182:49:writable] Warning: du2satp_der was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.
// "
// endResult

0 comments on commit 0f49428

Please sign in to comment.