Skip to content

Commit

Permalink
Forbid modifying protected elements (#8512)
Browse files Browse the repository at this point in the history
- Add check that protected elements are not modified.
- Refactor out the visibility propagation from Inst.instExtends to a
  separate function and do it earlier, so that elements have the correct
  visibility when applying modifiers.
  • Loading branch information
perost committed Feb 14, 2022
1 parent 70d7fff commit bf2fb17
Show file tree
Hide file tree
Showing 13 changed files with 234 additions and 34 deletions.
5 changes: 0 additions & 5 deletions .CI/compliance-newinst.failures
Expand Up @@ -135,7 +135,6 @@ ModelicaCompliance.Redeclare.ConstrainingType.RedeclareNonSubtypeComponentImpl
ModelicaCompliance.Redeclare.ConstrainingType.ReplaceableNonSubtypeComponent
ModelicaCompliance.Redeclare.ConstrainingType.ReplaceableNonSubtypeLongClass
ModelicaCompliance.Redeclare.ConstrainingType.ReplaceableNonSubtypeShortClass
ModelicaCompliance.Redeclare.Flattening.InheritanceProtectedClass
ModelicaCompliance.Redeclare.Restrictions.ConstantRedeclareElement
ModelicaCompliance.Redeclare.Restrictions.ConstantRedeclareModifier
ModelicaCompliance.Redeclare.Restrictions.DoubleRedeclareWithoutReplaceable
Expand Down Expand Up @@ -165,8 +164,4 @@ ModelicaCompliance.Scoping.NameLookup.Imports.RenamingImportNonPackage
ModelicaCompliance.Scoping.NameLookup.Imports.UnqualifiedImportProtected
ModelicaCompliance.Scoping.NameLookup.Simple.EnclosingClassLookupNonConstant
ModelicaCompliance.Scoping.NameLookup.Simple.EnclosingClassLookupShadowedConstant
ModelicaCompliance.Scoping.Visibility.ModifyProtectedClass
ModelicaCompliance.Scoping.Visibility.ModifyProtectedComp
ModelicaCompliance.Scoping.Visibility.RedeclareProtectedClass
ModelicaCompliance.Scoping.Visibility.RedeclareProtectedComp
ModelicaCompliance.Components.Variability.ConstantNoBinding
5 changes: 0 additions & 5 deletions .CI/compliance.failures
Expand Up @@ -135,7 +135,6 @@ ModelicaCompliance.Redeclare.ConstrainingType.RedeclareNonSubtypeComponentImpl
ModelicaCompliance.Redeclare.ConstrainingType.ReplaceableNonSubtypeComponent
ModelicaCompliance.Redeclare.ConstrainingType.ReplaceableNonSubtypeLongClass
ModelicaCompliance.Redeclare.ConstrainingType.ReplaceableNonSubtypeShortClass
ModelicaCompliance.Redeclare.Flattening.InheritanceProtectedClass
ModelicaCompliance.Redeclare.Restrictions.ConstantRedeclareElement
ModelicaCompliance.Redeclare.Restrictions.ConstantRedeclareModifier
ModelicaCompliance.Redeclare.Restrictions.DoubleRedeclareWithoutReplaceable
Expand Down Expand Up @@ -165,8 +164,4 @@ ModelicaCompliance.Scoping.NameLookup.Imports.RenamingImportNonPackage
ModelicaCompliance.Scoping.NameLookup.Imports.UnqualifiedImportProtected
ModelicaCompliance.Scoping.NameLookup.Simple.EnclosingClassLookupNonConstant
ModelicaCompliance.Scoping.NameLookup.Simple.EnclosingClassLookupShadowedConstant
ModelicaCompliance.Scoping.Visibility.ModifyProtectedClass
ModelicaCompliance.Scoping.Visibility.ModifyProtectedComp
ModelicaCompliance.Scoping.Visibility.RedeclareProtectedClass
ModelicaCompliance.Scoping.Visibility.RedeclareProtectedComp
ModelicaCompliance.Components.Variability.ConstantNoBinding
73 changes: 54 additions & 19 deletions OMCompiler/Compiler/NFFrontEnd/NFInst.mo
Expand Up @@ -889,8 +889,12 @@ algorithm
// Apply the modifiers of extends nodes.
ClassTree.mapExtends(cls_tree, function modifyExtends(scope = par));

// Propagate the visibility of extends to their elements.
ClassTree.mapExtends(cls_tree,
function applyExtendsVisibility(visibility = ExtendsVisibility.PUBLIC));

// Apply the modifiers of this scope.
applyModifier(mod, cls_tree, InstNode.name(node));
applyModifier(mod, cls_tree, node);

// Apply element redeclares.
ClassTree.mapRedeclareChains(cls_tree,
Expand All @@ -904,7 +908,6 @@ algorithm
// Instantiate the extends nodes.
ClassTree.mapExtends(cls_tree,
function instExtends(attributes = attributes, useBinding = useBinding,
visibility = ExtendsVisibility.PUBLIC,
instLevel = instLevel + 1, context = context));

// Instantiate local components.
Expand Down Expand Up @@ -966,7 +969,7 @@ algorithm
mod := instElementModifier(InstNode.definition(node), node, InstNode.parent(node));
outer_mod := Modifier.merge(outerMod, cls.modifier);
mod := Modifier.merge(outer_mod, mod);
applyModifier(mod, cls_tree, InstNode.name(node));
applyModifier(mod, cls_tree, node);

inst_cls := Class.INSTANCED_BUILTIN(ty, cls_tree, res);
node := InstNode.updateClass(inst_cls, node);
Expand Down Expand Up @@ -1125,20 +1128,16 @@ algorithm
end match;
end if;

applyModifier(ext_mod, cls_tree, InstNode.name(extendsNode));
applyModifier(ext_mod, cls_tree, extendsNode);
end modifyExtends;

type ExtendsVisibility = enumeration(PUBLIC, DERIVED_PROTECTED, PROTECTED);

function instExtends
function applyExtendsVisibility
input output InstNode node;
input Component.Attributes attributes;
input Boolean useBinding;
input ExtendsVisibility visibility;
input Integer instLevel;
input InstContext.Type context;
protected
Class cls, inst_cls;
Class cls;
ClassTree cls_tree;
ExtendsVisibility vis = visibility;
algorithm
Expand All @@ -1164,9 +1163,43 @@ algorithm
end for;
end if;

ClassTree.mapExtends(cls_tree, function applyExtendsVisibility(visibility = vis));
then
();

case Class.EXPANDED_DERIVED()
algorithm
if vis == ExtendsVisibility.PUBLIC and InstNode.isProtectedBaseClass(node) then
vis := ExtendsVisibility.DERIVED_PROTECTED;
end if;

cls.baseClass := applyExtendsVisibility(cls.baseClass, vis);
node := InstNode.updateClass(cls, node);
then
();

else ();
end match;
end applyExtendsVisibility;

function instExtends
input output InstNode node;
input Component.Attributes attributes;
input Boolean useBinding;
input Integer instLevel;
input InstContext.Type context;
protected
Class cls, inst_cls;
ClassTree cls_tree;
algorithm
cls := InstNode.getClass(node);

() := match cls
case Class.EXPANDED_CLASS(elements = cls_tree as ClassTree.INSTANTIATED_TREE())
algorithm
ClassTree.mapExtends(cls_tree,
function instExtends(attributes = attributes, useBinding = useBinding,
visibility = vis, instLevel = instLevel, context = context));
instLevel = instLevel, context = context));

ClassTree.applyLocalComponents(cls_tree,
function instComponent(attributes = attributes, innerMod = Modifier.NOMOD(),
Expand All @@ -1176,11 +1209,7 @@ algorithm

case Class.EXPANDED_DERIVED()
algorithm
if vis == ExtendsVisibility.PUBLIC and InstNode.isProtectedBaseClass(node) then
vis := ExtendsVisibility.DERIVED_PROTECTED;
end if;

cls.baseClass := instExtends(cls.baseClass, attributes, useBinding, vis, instLevel, context);
cls.baseClass := instExtends(cls.baseClass, attributes, useBinding, instLevel, context);
node := InstNode.updateClass(cls, node);
then
();
Expand All @@ -1201,7 +1230,7 @@ function applyModifier
each part with the relevant element in the scope."
input Modifier modifier;
input output ClassTree cls;
input String clsName;
input InstNode parent;
protected
list<Modifier> mods;
list<Mutable<InstNode>> node_ptrs;
Expand All @@ -1223,7 +1252,7 @@ algorithm
node := ClassTree.lookupElement(Modifier.name(mod), cls);
else
Error.addSourceMessage(Error.MISSING_MODIFIED_ELEMENT,
{Modifier.name(mod), clsName}, Modifier.info(mod));
{Modifier.name(mod), InstNode.name(parent)}, Modifier.info(mod));
fail();
end try;

Expand All @@ -1240,14 +1269,20 @@ algorithm
node_ptrs := ClassTree.lookupElementsPtr(Modifier.name(mod), cls);
else
Error.addSourceMessage(Error.MISSING_MODIFIED_ELEMENT,
{Modifier.name(mod), clsName}, Modifier.info(mod));
{Modifier.name(mod), InstNode.name(parent)}, Modifier.info(mod));
fail();
end try;

// Apply the modifier to each found node.
for node_ptr in node_ptrs loop
node := InstNode.resolveOuter(Mutable.access(node_ptr));

if InstNode.isProtected(node) and not InstNode.isExtends(parent) then
Error.addSourceMessage(Error.NF_MODIFY_PROTECTED,
{InstNode.name(node), Modifier.toString(mod)}, InstNode.info(node));
fail();
end if;

if InstNode.isComponent(node) then
InstNode.componentApply(node, Component.mergeModifier, mod);
else
Expand Down
14 changes: 11 additions & 3 deletions OMCompiler/Compiler/NFFrontEnd/NFInstNode.mo
Expand Up @@ -1443,9 +1443,6 @@ uniontype InstNode
output Boolean isProtected;
algorithm
isProtected := match node
local
SCode.Element def;

case CLASS_NODE(nodeType = InstNodeType.BASE_CLASS(definition =
SCode.Element.EXTENDS(visibility = SCode.Visibility.PROTECTED())))
then true;
Expand Down Expand Up @@ -1815,6 +1812,17 @@ uniontype InstNode
else Restriction.UNKNOWN();
end match;
end restriction;

function isExtends
input InstNode node;
output Boolean res;
algorithm
res := match node
case CLASS_NODE(definition = SCode.Element.EXTENDS()) then true;
case CLASS_NODE(nodeType = InstNodeType.BASE_CLASS(definition = SCode.Element.EXTENDS())) then true;
else false;
end match;
end isExtends;
end InstNode;

annotation(__OpenModelica_Interface="frontend");
Expand Down
4 changes: 3 additions & 1 deletion OMCompiler/Compiler/Util/Error.mo
Expand Up @@ -181,7 +181,7 @@ public constant ErrorTypes.Message INVALID_CONNECTOR_VARIABLE = ErrorTypes.MESSA
public constant ErrorTypes.Message TYPE_ERROR = ErrorTypes.MESSAGE(53, ErrorTypes.TRANSLATION(), ErrorTypes.ERROR(),
Gettext.gettext("Wrong type on %s, expected %s."));
public constant ErrorTypes.Message MODIFY_PROTECTED = ErrorTypes.MESSAGE(54, ErrorTypes.TRANSLATION(), ErrorTypes.WARNING(),
Gettext.gettext("Modification or redeclaration of protected elements is not allowed.\n\tElement: %s, modification: %s."));
Gettext.gettext("Modification or redeclaration of protected elements is not allowed.\n Element: %s, modification: %s."));
public constant ErrorTypes.Message INVALID_TUPLE_CONTENT = ErrorTypes.MESSAGE(55, ErrorTypes.TRANSLATION(), ErrorTypes.ERROR(),
Gettext.gettext("Tuple %s must contain component references only."));
public constant ErrorTypes.Message MISSING_REDECLARE_IN_CLASS_MOD = ErrorTypes.MESSAGE(56, ErrorTypes.TRANSLATION(), ErrorTypes.ERROR(),
Expand Down Expand Up @@ -864,6 +864,8 @@ public constant ErrorTypes.Message FUNCTION_ARGUMENT_MUST_BE = ErrorTypes.MESSAG
Gettext.gettext("The argument to ‘%s‘ must be %s."));
public constant ErrorTypes.Message UNEXPECTED_COMPONENT_IN_COMPOSITE_NAME = ErrorTypes.MESSAGE(395, ErrorTypes.TRANSLATION(), ErrorTypes.ERROR(),
Gettext.gettext("Found component ‘%s‘ in composite name ‘%s‘, expected class."));
public constant ErrorTypes.Message NF_MODIFY_PROTECTED = ErrorTypes.MESSAGE(54, ErrorTypes.TRANSLATION(), ErrorTypes.ERROR(),
Gettext.gettext("Protected element ‘%s‘ may not be modified, got ‘%s‘."));

public constant ErrorTypes.Message INITIALIZATION_NOT_FULLY_SPECIFIED = ErrorTypes.MESSAGE(496, ErrorTypes.TRANSLATION(), ErrorTypes.WARNING(),
Gettext.gettext("The initial conditions are not fully specified. %s."));
Expand Down
8 changes: 7 additions & 1 deletion testsuite/flattening/modelica/scodeinst/Makefile
Expand Up @@ -870,6 +870,13 @@ Prefix2.mo \
Prefix3.mo \
PrintRecordTypes1.mo \
PropagateExtends.mo \
ProtectedMod1.mo \
ProtectedMod2.mo \
ProtectedMod3.mo \
ProtectedMod4.mo \
ProtectedMod5.mo \
ProtectedMod6.mo \
ProtectedMod7.mo \
RangeInvalidStep1.mo \
RangeInvalidStep2.mo \
RangeInvalidStep3.mo \
Expand Down Expand Up @@ -1095,7 +1102,6 @@ IfExpression6.mo \
IfExpression9.mo \
RedeclareConstant1.mo \
Connect3.mo \
ProtectedMod1.mo \
dim2.mo \
DuplicateElements6.mo \
DuplicateElements7.mo \
Expand Down
10 changes: 10 additions & 0 deletions testsuite/flattening/modelica/scodeinst/ProtectedMod1.mo
Expand Up @@ -12,3 +12,13 @@ end A;
model ProtectedMod1
A a(x = 2.0);
end ProtectedMod1;

// Result:
// Error processing file: ProtectedMod1.mo
// [flattening/modelica/scodeinst/ProtectedMod1.mo:9:13-9:25:writable] Error: Protected element ‘x‘ may not be modified, got ‘x = 2.0‘.
//
// # Error encountered! Exiting...
// # Please check the error message and the flags.
//
// Execution failed!
// endResult
20 changes: 20 additions & 0 deletions testsuite/flattening/modelica/scodeinst/ProtectedMod2.mo
@@ -0,0 +1,20 @@
// name: ProtectedMod2
// keywords:
// status: correct
// cflags: -d=newInst
//
//

model A
protected Real x = 1.0;
end A;

model ProtectedMod2
extends A(x = 2.0);
end ProtectedMod2;

// Result:
// class ProtectedMod2
// protected Real x = 2.0;
// end ProtectedMod2;
// endResult
28 changes: 28 additions & 0 deletions testsuite/flattening/modelica/scodeinst/ProtectedMod3.mo
@@ -0,0 +1,28 @@
// name: ProtectedMod3
// keywords:
// status: incorrect
// cflags: -d=newInst
//
//

model A
protected Real x = 1.0;
end A;

model B
A a;
end B;

model ProtectedMod3
extends B(a(x = 2.0));
end ProtectedMod3;

// Result:
// Error processing file: ProtectedMod3.mo
// [flattening/modelica/scodeinst/ProtectedMod3.mo:9:13-9:25:writable] Error: Protected element ‘x‘ may not be modified, got ‘x = 2.0‘.
//
// # Error encountered! Exiting...
// # Please check the error message and the flags.
//
// Execution failed!
// endResult
26 changes: 26 additions & 0 deletions testsuite/flattening/modelica/scodeinst/ProtectedMod4.mo
@@ -0,0 +1,26 @@
// name: ProtectedMod4
// keywords:
// status: incorrect
// cflags: -d=newInst
//
//

model A
protected Real x = 1.0;
end A;

model B = A;

model ProtectedMod4
B b(x = 1.0);
end ProtectedMod4;

// Result:
// Error processing file: ProtectedMod4.mo
// [flattening/modelica/scodeinst/ProtectedMod4.mo:9:13-9:25:writable] Error: Protected element ‘x‘ may not be modified, got ‘x = 1.0‘.
//
// # Error encountered! Exiting...
// # Please check the error message and the flags.
//
// Execution failed!
// endResult
25 changes: 25 additions & 0 deletions testsuite/flattening/modelica/scodeinst/ProtectedMod5.mo
@@ -0,0 +1,25 @@
// name: ProtectedMod5
// keywords:
// status: correct
// cflags: -d=newInst
//
//

model A
Real x = 1.0;
end A;

model B
protected
A a;
end B;

model ProtectedMod5
extends B(a(x = 2.0));
end ProtectedMod5;

// Result:
// class ProtectedMod5
// protected Real a.x = 2.0;
// end ProtectedMod5;
// endResult

0 comments on commit bf2fb17

Please sign in to comment.