Skip to content

Commit

Permalink
Constants validation (fb instances not allowed #295, scoped const exp…
Browse files Browse the repository at this point in the history
…ressions #292) (#306)

* validate illegal fb and class consts

fb-instances and class-instances cannot be declared
as constants.

the implementation does not allow to declare a const
of type FB or Class, an array of FBs or Classes
or a struct that contains a field of type FB or
Class.

Furthermore the DataTypeInformation::Struct now contains
a new field: 'source' that indicates where this struct
comes from. It is either an original Declaration
(TYPE x : STRUCT ... END_TYPE) or it was generated
from a POU(PouType). With this field it is now very
easy to see what a DataType offers (is it an FB or Class,
can I call it, etc.)

closes #295

* add scope to unresolved const expressions

the additional scope parameter in const_evaluator::evaluate(...)
helps the evaluator to distinguish between global constants and
POU-local ones. When resolving const expressions the given
scope will be used to resolve references.

therefore every ConstExpression holds an optional scope when it
is registered (see enum ConstExpression).

closes #292
  • Loading branch information
riederm committed Sep 27, 2021
1 parent 5f2dbcd commit 2f3d14f
Show file tree
Hide file tree
Showing 11 changed files with 291 additions and 53 deletions.
3 changes: 2 additions & 1 deletion src/codegen/generators/struct_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ impl<'a, 'b> StructGenerator<'a, 'b> {
{
Some(statement) => {
//evalute the initializer to a value
//TODO we should not resolve here, they should be resolved before!
let evaluated_const =
crate::resolver::const_evaluator::evaluate(statement, self.index);
crate::resolver::const_evaluator::evaluate(statement, None, self.index);
match evaluated_const {
Ok(Some(initializer)) => {
//create the appropriate Literal AST-Statement
Expand Down
17 changes: 10 additions & 7 deletions src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub enum DataTypeType {
AliasType, // a Custom-Alias-dataType
}

#[derive(Clone, PartialEq)]
#[derive(Clone, PartialEq, Debug)]
pub enum ImplementationType {
Program,
Function,
Expand All @@ -105,7 +105,7 @@ pub enum ImplementationType {
Method,
}

#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct ImplementationIndexEntry {
call_name: String,
type_name: String,
Expand Down Expand Up @@ -355,9 +355,9 @@ impl Index {
initializer_id
.as_ref()
.and_then(|it| import_from.remove(it))
.map(|(init, target_type)| {
.map(|(init, target_type, scope)| {
self.get_mut_const_expressions()
.add_constant_expression(init, target_type)
.add_constant_expression(init, target_type, scope)
})
}

Expand All @@ -372,9 +372,12 @@ impl Index {
TypeSize::LiteralInteger(_) => type_size.clone(),
TypeSize::ConstExpression(id) => import_from
.remove(id)
.map(|(expr, target_type)| {
self.get_mut_const_expressions()
.add_constant_expression(expr, target_type)
.map(|(expr, target_type, scope)| {
self.get_mut_const_expressions().add_constant_expression(
expr,
target_type,
scope,
)
})
.map(TypeSize::from_expression)
.unwrap(),
Expand Down
72 changes: 57 additions & 15 deletions src/index/const_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ struct ConstWrapper {
impl ConstWrapper {
pub fn get_statement(&self) -> &AstStatement {
match &self.expr {
ConstExpression::Unresolved(statement) => statement,
ConstExpression::Unresolved { statement, .. } => statement,
ConstExpression::Resolved(statement) => statement,
ConstExpression::Unresolvable { statement, .. } => statement,
}
Expand All @@ -29,7 +29,13 @@ impl ConstWrapper {
/// whether this expression was already (potentially) resolved or not, or if a
/// resolving failed.
pub enum ConstExpression {
Unresolved(AstStatement),
Unresolved {
statement: AstStatement,
/// optional qualifier used when evaluating this expression
/// e.g. a const-expression inside a POU would use this POU's name as a
/// qualifier.
scope: Option<String>,
},
Resolved(AstStatement),
Unresolvable {
statement: AstStatement,
Expand All @@ -38,13 +44,23 @@ pub enum ConstExpression {
}

impl ConstExpression {
/// returns the const-expression represented as an AST-element
pub fn get_statement(&self) -> &AstStatement {
match &self {
ConstExpression::Unresolved(statement) => statement,
ConstExpression::Unresolved { statement, .. } => statement,
ConstExpression::Resolved(statement) => statement,
ConstExpression::Unresolvable { statement, .. } => statement,
}
}

/// returns an optional qualifier that should be used as a scope when
/// resolving this ConstExpression (e.g. the host's POU-name)
pub fn get_qualifier(&self) -> Option<&str> {
match &self {
ConstExpression::Unresolved { scope, .. } => scope.as_ref().map(|it| it.as_str()),
_ => None,
}
}
}

#[derive(Default)]
Expand All @@ -59,15 +75,29 @@ impl ConstExpressions {
}
}

pub fn add_expression(&mut self, statement: AstStatement, target_type_name: String) -> ConstId {
/// adds the const expression `statement`
/// - `statement`: the const expression to add
/// - `target_type_name`: the datatype this expression will be assigned to
/// - `scope`: the scope this expression needs to be resolved in (e.g. a POU's name)
pub fn add_expression(
&mut self,
statement: AstStatement,
target_type_name: String,
scope: Option<String>,
) -> ConstId {
self.expressions.insert(ConstWrapper {
expr: ConstExpression::Unresolved(statement),
expr: ConstExpression::Unresolved { statement, scope },
target_type_name,
})
}

pub fn find_expression(&self, id: &ConstId) -> Option<&AstStatement> {
self.expressions.get(*id).map(|it| it.get_statement())
/// returns the expression associated with the given `id` together with an optional
/// `qualifier` that represents the expressions scope (e.g. the host's POU-name)
pub fn find_expression(&self, id: &ConstId) -> (Option<&AstStatement>, Option<&str>) {
self.expressions
.get(*id)
.map(|it| (Some(it.expr.get_statement()), it.expr.get_qualifier()))
.unwrap_or((None, None))
}

pub fn find_expression_target_type(&self, id: &ConstId) -> Option<&str> {
Expand All @@ -83,11 +113,14 @@ impl ConstExpressions {
self.expressions.get(*id).map(|it| &it.expr)
}

pub fn remove(&mut self, id: &ConstId) -> Option<(AstStatement, String)> {
/// removes the expression from the ConstExpressions and returns all of its elements
pub fn remove(&mut self, id: &ConstId) -> Option<(AstStatement, String, Option<String>)> {
self.expressions.remove(*id).map(|it| match it.expr {
ConstExpression::Unresolved(s) => (s, it.target_type_name),
ConstExpression::Resolved(s) => (s, it.target_type_name),
ConstExpression::Unresolvable { statement: s, .. } => (s, it.target_type_name),
ConstExpression::Unresolved { statement, scope } => {
(statement, it.target_type_name, scope)
}
ConstExpression::Resolved(s) => (s, it.target_type_name, None),
ConstExpression::Unresolvable { statement: s, .. } => (s, it.target_type_name, None),
})
}

Expand Down Expand Up @@ -119,8 +152,16 @@ impl ConstExpressions {
}

/// adds the given constant expression to the constants arena and returns the ID to reference it
pub fn add_constant_expression(&mut self, expr: AstStatement, target_type: String) -> ConstId {
self.add_expression(expr, target_type)
/// - `expr`: the const expression to add
/// - `target_type`: the datatype this expression will be assigned to
/// - `scope`: the scope this expression needs to be resolved in (e.g. a POU's name)
pub fn add_constant_expression(
&mut self,
expr: AstStatement,
target_type: String,
scope: Option<String>,
) -> ConstId {
self.add_expression(expr, target_type, scope)
}

/// convinience-method to add the constant exression if there is some, otherwhise not
Expand All @@ -130,8 +171,9 @@ impl ConstExpressions {
&mut self,
expr: Option<AstStatement>,
targe_type_name: &str,
scope: Option<String>,
) -> Option<ConstId> {
expr.map(|it| self.add_constant_expression(it, targe_type_name.to_string()))
expr.map(|it| self.add_constant_expression(it, targe_type_name.to_string(), scope))
}

/// convinience-method to query for an optional constant expression.
Expand All @@ -144,7 +186,7 @@ impl ConstExpressions {

/// query the constants arena for an expression associated with the given `id`
pub fn get_constant_statement(&self, id: &ConstId) -> Option<&AstStatement> {
self.find_expression(id)
self.find_expression(id).0
}

/// query the constants arena for an expression that can be evaluated to an i128.
Expand Down
8 changes: 2 additions & 6 deletions src/index/tests/index_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,7 @@ fn fb_methods_are_indexed() {
.unwrap()
.get_type_information();
if let crate::typesystem::DataTypeInformation::Struct {
name,
member_names,
varargs: _,
name, member_names, ..
} = info
{
assert_eq!("myFuncBlock.foo_interface", name);
Expand Down Expand Up @@ -216,9 +214,7 @@ fn class_methods_are_indexed() {
.unwrap()
.get_type_information();
if let crate::typesystem::DataTypeInformation::Struct {
name,
member_names,
varargs: _,
name, member_names, ..
} = info
{
assert_eq!("myClass.foo_interface", name);
Expand Down
34 changes: 27 additions & 7 deletions src/index/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@ pub fn visit_pou(index: &mut Index, pou: &Pou) {
};
let initial_value = index
.get_mut_const_expressions()
.maybe_add_constant_expression(var.initializer.clone(), type_name.as_str());
.maybe_add_constant_expression(
var.initializer.clone(),
type_name.as_str(),
Some(pou.name.clone()),
);

index.register_member_variable(
&MemberInfo {
Expand Down Expand Up @@ -129,6 +133,7 @@ pub fn visit_pou(index: &mut Index, pou: &Pou) {
name: interface_name,
member_names,
varargs,
source: StructSource::Pou(pou.pou_type.clone()),
},
);
}
Expand Down Expand Up @@ -178,7 +183,7 @@ fn visit_global_var_block(index: &mut Index, block: &VariableBlock) {
let target_type = var.data_type.get_name().unwrap_or_default();
let initializer = index
.get_mut_const_expressions()
.maybe_add_constant_expression(var.initializer.clone(), target_type);
.maybe_add_constant_expression(var.initializer.clone(), target_type, None);
index.register_global_variable(
&var.name,
var.data_type.get_name().unwrap(),
Expand Down Expand Up @@ -215,13 +220,15 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
name: type_name.clone(),
member_names,
varargs: None,
source: StructSource::OriginalDeclaration,
};

let init = index
.get_mut_const_expressions()
.maybe_add_constant_expression(
type_declatation.initializer.clone(),
type_name.as_str(),
None,
);
index.register_type(name.as_ref().unwrap(), init, information);
for (count, var) in variables.iter().enumerate() {
Expand All @@ -240,7 +247,7 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
let member_type = var.data_type.get_name().unwrap();
let init = index
.get_mut_const_expressions()
.maybe_add_constant_expression(var.initializer.clone(), member_type);
.maybe_add_constant_expression(var.initializer.clone(), member_type, None);
index.register_member_variable(
&MemberInfo {
container_name: struct_name,
Expand Down Expand Up @@ -268,6 +275,7 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
.maybe_add_constant_expression(
type_declatation.initializer.clone(),
enum_name.as_str(),
None,
);
index.register_type(enum_name.as_str(), init, information);

Expand All @@ -279,6 +287,7 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
id: 0,
},
typesystem::INT_TYPE.to_string(),
None,
);
index.register_enum_element(
v,
Expand Down Expand Up @@ -313,6 +322,7 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
.maybe_add_constant_expression(
type_declatation.initializer.clone(),
name.as_ref().unwrap(),
None,
);
index.register_type(name.as_ref().unwrap(), init, information)
}
Expand All @@ -332,12 +342,14 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
constants.add_constant_expression(
*start.clone(),
typesystem::INT_TYPE.to_string(),
None,
),
),
end_offset: TypeSize::from_expression(
constants.add_constant_expression(
*end.clone(),
typesystem::INT_TYPE.to_string(),
None,
),
),
})
Expand All @@ -362,6 +374,7 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
.maybe_add_constant_expression(
type_declatation.initializer.clone(),
name.as_ref().unwrap(),
None,
);
index.register_type(name.as_ref().unwrap(), init, information)
}
Expand All @@ -382,6 +395,7 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
.maybe_add_constant_expression(
type_declatation.initializer.clone(),
name.as_ref().unwrap(),
None,
);
index.register_type(name.as_ref().unwrap(), init, information)
}
Expand Down Expand Up @@ -416,17 +430,23 @@ fn visit_data_type(index: &mut Index, type_declatation: &UserTypeDeclaration) {
};

TypeSize::from_expression(
index
.get_mut_const_expressions()
.add_constant_expression(len_plus_1, type_name.clone()),
index.get_mut_const_expressions().add_constant_expression(
len_plus_1,
type_name.clone(),
None,
),
)
}
None => TypeSize::from_literal(DEFAULT_STRING_LEN + 1),
};
let information = DataTypeInformation::String { size, encoding };
let init = index
.get_mut_const_expressions()
.maybe_add_constant_expression(type_declatation.initializer.clone(), type_name);
.maybe_add_constant_expression(
type_declatation.initializer.clone(),
type_name,
None,
);
index.register_type(name.as_ref().unwrap(), init, information)
}
DataType::VarArgs { .. } => {} //Varargs are not indexed
Expand Down
10 changes: 9 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ pub enum ErrNo {
//variable related
var__unresolved_constant,
var__invalid_constant_block,
var__invalid_constant,
var__cannot_assign_to_const,

//reference related
reference__unresolved,
//variable related

//type related
type__literal_out_of_range,
Expand Down Expand Up @@ -286,6 +286,14 @@ impl Diagnostic {
}
}

pub fn invalid_constant(constant_name: &str, location: SourceRange) -> Diagnostic {
Diagnostic::SyntaxError {
message: format!("Invalid constant {:} - Functionblock- and Class-instances cannot be delcared constant", constant_name),
range: location,
err_no: ErrNo::var__invalid_constant,
}
}

pub fn cannot_assign_to_constant(qualified_name: &str, location: SourceRange) -> Diagnostic {
Diagnostic::SyntaxError {
message: format!("Cannot assign to CONSTANT '{:}'", qualified_name),
Expand Down
Loading

0 comments on commit 2f3d14f

Please sign in to comment.