feat(cfc): Conditional Returns (#950)

Author: volsa

Cargo.lock

@@ -46,6 +46,7 @@ insta = "1.31.0"
 pretty_assertions = "1.3.0"
 driver = { path = "./compiler/plc_driver/", package = "plc_driver" }
 project = { path = "./compiler/plc_project/", package = "plc_project" }
+plc_xml = { path = "./compiler/plc_xml" }
 serial_test = "*"
 tempfile = "3"
 encoding_rs.workspace = true

Cargo.toml

@@ -10,6 +10,7 @@ use serde::{Deserialize, Serialize};
 use crate::{
     control_statements::{
         AstControlStatement, CaseStatement, ConditionalBlock, ForLoopStatement, IfStatement, LoopStatement,
+        ReturnStatement,
     },
     literals::{AstLiteral, StringValue},
     pre_processor,
@@ -614,7 +615,7 @@ pub enum AstStatement {
     CaseCondition(Box<AstNode>),
     ExitStatement(()),
     ContinueStatement(()),
-    ReturnStatement(()),
+    ReturnStatement(ReturnStatement),
 }
 
 impl Debug for AstNode {
@@ -714,7 +715,9 @@ impl Debug for AstNode {
             AstStatement::CaseCondition(condition) => {
                 f.debug_struct("CaseCondition").field("condition", condition).finish()
             }
-            AstStatement::ReturnStatement(..) => f.debug_struct("ReturnStatement").finish(),
+            AstStatement::ReturnStatement(ReturnStatement { condition }) => {
+                f.debug_struct("ReturnStatement").field("condition", condition).finish()
+            }
             AstStatement::ContinueStatement(..) => f.debug_struct("ContinueStatement").finish(),
             AstStatement::ExitStatement(..) => f.debug_struct("ExitStatement").finish(),
             AstStatement::CastStatement(CastStatement { target, type_name }) => {
@@ -1058,8 +1061,13 @@ impl AstFactory {
         // AstStatement::EmptyStatement (  EmptyStatement {}, location, id }
     }
 
-    pub fn create_return_statement(location: SourceLocation, id: AstId) -> AstNode {
-        AstNode { stmt: AstStatement::ReturnStatement(()), location, id }
+    pub fn create_return_statement(
+        condition: Option<AstNode>,
+        location: SourceLocation,
+        id: AstId,
+    ) -> AstNode {
+        let condition = condition.map(Box::new);
+        AstNode { stmt: AstStatement::ReturnStatement(ReturnStatement { condition }), location, id }
     }
 
     pub fn create_exit_statement(location: SourceLocation, id: AstId) -> AstNode {
@@ -1210,8 +1218,7 @@ impl AstFactory {
     }
 
     /// creates a not-expression
-    pub fn create_not_expression(operator: AstNode, location: SourceLocation) -> AstNode {
-        let id = operator.get_id();
+    pub fn create_not_expression(operator: AstNode, location: SourceLocation, id: AstId) -> AstNode {
         AstNode {
             stmt: AstStatement::UnaryExpression(UnaryExpression {
                 value: Box::new(operator),

compiler/plc_ast/src/ast.rs

@@ -46,6 +46,13 @@ pub struct ConditionalBlock {
     pub body: Vec<AstNode>,
 }
 
+#[derive(Clone, PartialEq)]
+pub struct ReturnStatement {
+    /// Indicates that the given condition must evaluate to true in order for the return to take place.
+    /// Only used in CFC where the condition may be [`Some`] and [`None`] otherwise.
+    pub condition: Option<Box<AstNode>>,
+}
+
 impl Debug for ConditionalBlock {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("ConditionalBlock")

compiler/plc_ast/src/control_statements.rs

@@ -719,6 +719,33 @@ impl Diagnostic {
     }
 }
 
+// CFC related diagnostics
+impl Diagnostic {
+    pub fn empty_control_statement(range: SourceLocation) -> Diagnostic {
+        Diagnostic::SemanticError {
+            message: "Control statement has no connection".to_string(),
+            range: vec![range],
+            err_no: ErrNo::cfc__empty_control_statement,
+        }
+    }
+
+    pub fn undefined_node(local_id: usize, ref_local_id: usize, range: SourceLocation) -> Diagnostic {
+        Diagnostic::SemanticError {
+            message: format!("Node {local_id} is referencing a non-existing element with ID {ref_local_id}"),
+            range: vec![range],
+            err_no: ErrNo::cfc__undefined_node,
+        }
+    }
+
+    pub fn unexpected_nodes(range: SourceLocation) -> Diagnostic {
+        Diagnostic::SemanticError {
+            message: "Unexpected relationship between nodes".to_string(),
+            range: vec![range],
+            err_no: ErrNo::cfc__unexpected_node,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use codespan_reporting::files::{Location, SimpleFile};

compiler/plc_diagnostics/src/diagnostics.rs

@@ -87,6 +87,11 @@ pub enum ErrNo {
     case__duplicate_condition,
     case__case_condition_outside_case_statement,
     case__invalid_case_condition,
+
+    // CFC related
+    cfc__empty_control_statement,
+    cfc__undefined_node,
+    cfc__unexpected_node,
 }
 
 impl Display for ErrNo {

compiler/plc_diagnostics/src/errno.rs

@@ -13,8 +13,8 @@ indexmap = "1.9"
 html-escape = "0.2"
 plc = { path = "../..", package = "rusty" }
 ast = { path = "../plc_ast/", package = "plc_ast" }
-plc_diagnostics = { path = "../plc_diagnostics"}
-plc_source = { path = "../plc_source"}
+plc_diagnostics = { path = "../plc_diagnostics" }
+plc_source = { path = "../plc_source" }
 
 [features]
 default = []

compiler/plc_xml/Cargo.toml

@@ -65,8 +65,21 @@ impl Parseable for Control {
         loop {
             match reader.peek()? {
                 Event::Start(tag) | Event::Empty(tag) => match tag.name().as_ref() {
-                    b"negated" => attributes.extend(reader.attributes()?),
                     b"connection" => attributes.extend(reader.attributes()?),
+
+                    // As opposed to e.g. variables where the negation information is directly stored in its
+                    // attributes (e.g. `<inVariable negated="false" .../>`) return elements store their
+                    // negation information in a seperate nested element called `negated` with the form of
+                    // `<negated value="..."/>`.
+                    // Hence we search for a negate element and extract its information from their attributes.
+                    b"negated" => {
+                        let value = reader.attributes()?;
+                        attributes.insert(
+                            "negated".to_string(),
+                            (value.get_or_err("value")? == "true").to_string(),
+                        );
+                    }
+
                     _ => reader.consume()?,
                 },
 

compiler/plc_xml/src/model/control.rs

@@ -145,6 +145,16 @@ macro_rules! declare_type_and_extend_if_needed {
                     self
                 }
 
+                pub fn with_local_id(mut self, value: &'static str) -> Self {
+                    self.0.attributes.push(("localId", value));
+                    self
+                }
+
+                pub fn with_execution_order_id(mut self, value: &'static str) -> Self {
+                    self.0.attributes.push(("executionOrderId", value));
+                    self
+                }
+
                 pub fn with_data(mut self, data: &'static str) -> Self {
                     self.0.content = Content::Data(data);
                     self
@@ -210,7 +220,8 @@ declare_type_and_extend_if_needed! {
         (XInVariable, with_in_variable),
         (XOutVariable, with_out_variable),
         (XContinuation, with_continuation),
-        (XConnector, with_connector)
+        (XConnector, with_connector),
+        (XReturn, with_return)
     ),
     (
         XVariable, "variable",
@@ -253,7 +264,8 @@ declare_type_and_extend_if_needed! {
     ),
     (
         XData, "data",
-        (XTextDeclaration, with_text_declaration)
+        (XTextDeclaration, with_text_declaration),
+        (XNegated, with_negated_field)
     ),
     (
         XTextDeclaration, "textDeclaration",
@@ -277,6 +289,14 @@ declare_type_and_extend_if_needed! {
         (XPosition, with_position),
         (XConnectionPointIn, with_connection_point_in)
     ),
+    (
+        XReturn, "return",
+        (XConnectionPointIn, with_connection_point_in),
+        (XAddData, with_add_data)
+    ),
+    (
+        XNegated, "negated",
+    ),
 }
 
 #[cfg(test)]
@@ -285,8 +305,8 @@ mod tests {
     use crate::serializer::{XConnection, XConnectionPointIn, XConnectionPointOut, XRelPosition};
 
     use super::{
-        XAddData, XBlock, XContent, XData, XInVariable, XInterface, XOutVariable, XPou, XTextDeclaration,
-        XVariable,
+        XAddData, XBlock, XContent, XData, XInVariable, XInterface, XNegated, XOutVariable, XPou,
+        XTextDeclaration, XVariable,
     };
 
     // convenience methods to reduce amount of boiler-plate-code
@@ -376,4 +396,12 @@ mod tests {
             ))
         }
     }
+
+    impl XAddData {
+        pub(crate) fn negated(is_negated: bool) -> Self {
+            XAddData::new().with_data_data(XData::new().with_negated_field(
+                XNegated::new().with_attribute("value", if is_negated { "true" } else { "false" }),
+            ))
+        }
+    }
 }

compiler/plc_xml/src/serializer.rs

@@ -19,6 +19,7 @@ use crate::{
 
 mod action;
 mod block;
+mod control;
 mod fbd;
 mod pou;
 #[cfg(test)]
@@ -60,24 +61,28 @@ fn parse(
     linkage: LinkageType,
     id_provider: IdProvider,
 ) -> (CompilationUnit, Vec<Diagnostic>) {
-    // transform the xml file to a data model.
+    // Transform the xml file to a data model.
     // XXX: consecutive call-statements are nested in a single ast-statement. this will be broken up with temporary variables in the future
-    let Ok(project) = visit(&source.source) else {
-        todo!("cfc errors need to be transformed into diagnostics")
+    let project = match visit(&source.source) {
+        Ok(project) => project,
+        Err(why) => todo!("cfc errors need to be transformed into diagnostics; {why:?}"),
     };
 
-    // create a new parse session
+    // Create a new parse session
     let source_location_factory = SourceLocationFactory::for_source(source);
     let parser =
         ParseSession::new(&project, source.get_location_str(), id_provider, linkage, source_location_factory);
 
-    // try to parse a declaration data field
-    let Some((unit, diagnostics)) = parser.try_parse_declaration() else {
+    // Parse the declaration data field
+    let Some((unit, mut diagnostics)) = parser.try_parse_declaration() else {
         unimplemented!("XML schemas without text declarations are not yet supported")
     };
 
-    // transform the data model into rusty AST statements and add them to the compilation unit
-    (unit.with_implementations(parser.parse_model()), diagnostics)
+    // Transform the data-model into an AST
+    let (implementations, parser_diagnostics) = parser.parse_model();
+    diagnostics.extend(parser_diagnostics);
+
+    (unit.with_implementations(implementations), diagnostics)
 }
 
 pub(crate) struct ParseSession<'parse> {
@@ -86,6 +91,7 @@ pub(crate) struct ParseSession<'parse> {
     linkage: LinkageType,
     file_name: &'static str,
     range_factory: SourceLocationFactory,
+    diagnostics: Vec<Diagnostic>,
 }
 
 impl<'parse> ParseSession<'parse> {
@@ -96,7 +102,7 @@ impl<'parse> ParseSession<'parse> {
         linkage: LinkageType,
         range_factory: SourceLocationFactory,
     ) -> Self {
-        ParseSession { project, id_provider, linkage, file_name, range_factory }
+        ParseSession { project, id_provider, linkage, file_name, range_factory, diagnostics: Vec::new() }
     }
 
     /// parse the compilation unit from the addData field
@@ -130,15 +136,16 @@ impl<'parse> ParseSession<'parse> {
         exp
     }
 
-    fn parse_model(&self) -> Vec<Implementation> {
+    fn parse_model(mut self) -> (Vec<Implementation>, Vec<Diagnostic>) {
         let mut implementations = vec![];
         for pou in &self.project.pous {
             // transform body
-            implementations.push(pou.build_implementation(self));
+            implementations.push(pou.build_implementation(&mut self));
             // transform actions
-            pou.actions.iter().for_each(|action| implementations.push(action.build_implementation(self)));
+            pou.actions.iter().for_each(|action| implementations.push(action.build_implementation(&self)));
         }
-        implementations
+
+        (implementations, self.diagnostics)
     }
 
     fn next_id(&self) -> AstId {