Introduce (proof-of-concept) ValidationParticipant

compiler/plc_diagnostics/src/diagnostician.rs

@@ -19,6 +19,9 @@ pub struct Diagnostician {
     filename_fileid_mapping: FxHashMap<String, usize>,
 }
 
+unsafe impl Send for Diagnostician {}
+unsafe impl Sync for Diagnostician {}
+
 impl Diagnostician {
     /// registers the given source-code at the diagnostician, so it can
     /// preview errors in the source

compiler/plc_diagnostics/src/reporter.rs

@@ -8,7 +8,7 @@ pub mod null;
 
 /// the DiagnosticReporter decides on the format and where to report the diagnostic to.
 /// possible implementations could print to either std-out, std-err or a file, etc.
-pub trait DiagnosticReporter {
+pub trait DiagnosticReporter: Sync + Send {
     /// reports the given diagnostic
     fn report(&mut self, diagnostics: &[ResolvedDiagnostics]);
     /// register the given path & src and returns an ID to indicate

compiler/plc_driver/src/lib.rs

@@ -10,7 +10,7 @@
 
 use anyhow::{anyhow, Result};
 use pipelines::{
-    participant::{CodegenParticipant, InitParticipant},
+    participant::{CodegenParticipant, InitParticipant, ValidationParticipant},
     AnnotatedProject, BuildPipeline, GeneratedProject, Pipeline,
 };
 use std::{
@@ -163,6 +163,7 @@ pub fn compile<T: AsRef<str> + AsRef<OsStr> + Debug>(args: &[T]) -> Result<()> {
         libraries: pipeline.project.get_libraries().to_vec(),
     };
     pipeline.register_participant(Box::new(codegen_participant));
+    pipeline.register_participant(Box::new(ValidationParticipant::new(pipeline.diagnostician.clone())));
     let init_participant =
         InitParticipant::new(&pipeline.project.get_init_symbol_name(), pipeline.context.provider());
     pipeline.register_mut_participant(Box::new(init_participant));
@@ -193,7 +194,7 @@ pub fn parse_and_annotate<T: SourceContainer + Clone>(
     let mut pipeline = BuildPipeline {
         context,
         project,
-        diagnostician: Diagnostician::default(),
+        diagnostician: Arc::new(RwLock::new(Diagnostician::default())),
         compile_parameters: None,
         linker: LinkerType::Internal,
         mutable_participants: Vec::default(),
@@ -223,14 +224,13 @@ fn generate_to_string_internal<T: SourceContainer>(
     // plc src --ir --single-module
     let project = Project::new(name.to_string()).with_sources(src);
     let context = GlobalContext::new().with_source(project.get_sources(), None)?;
-    let diagnostician = Diagnostician::default();
     let mut params = cli::CompileParameters::parse(&["--ir", "--single-module", "-O", "none"])
         .map_err(|e| Diagnostic::new(e.to_string()))?;
     params.generate_debug = debug;
     let mut pipeline = BuildPipeline {
         context,
         project,
-        diagnostician,
+        diagnostician: Arc::new(RwLock::new(Diagnostician::default())),
         compile_parameters: Some(params),
         linker: LinkerType::Internal,
         mutable_participants: Vec::default(),
@@ -241,7 +241,7 @@ fn generate_to_string_internal<T: SourceContainer>(
     let project = pipeline.annotate(project)?;
     // Validate
     // TODO: move validation to participants, maybe refactor codegen to stop at generated modules and persist in dedicated step?
-    project.validate(&pipeline.context, &mut pipeline.diagnostician)?;
+    project.validate(&pipeline.context, pipeline.diagnostician.clone())?;
     let context = CodegenContext::create();
     let module =
         project.generate_single_module(&context, pipeline.get_compile_options().as_ref().unwrap())?;

compiler/plc_driver/src/pipelines.rs

@@ -5,7 +5,7 @@ use std::{
     fs::{self, File},
     io::Write,
     path::{Path, PathBuf},
-    sync::Mutex,
+    sync::{Arc, Mutex, RwLock},
 };
 
 use crate::{
@@ -54,7 +54,7 @@ pub mod participant;
 pub struct BuildPipeline<T: SourceContainer> {
     pub context: GlobalContext,
     pub project: Project<T>,
-    pub diagnostician: Diagnostician,
+    pub diagnostician: Arc<RwLock<Diagnostician>>,
     pub compile_parameters: Option<CompileParameters>,
     pub linker: LinkerType,
     pub mutable_participants: Vec<Box<dyn PipelineParticipantMut>>,
@@ -98,12 +98,15 @@ impl TryFrom<CompileParameters> for BuildPipeline<PathBuf> {
             ErrorFormat::Clang => Diagnostician::clang_format_diagnostician(),
             ErrorFormat::None => Diagnostician::null_diagnostician(),
         };
+
         let diagnostician = if let Some(configuration) = compile_parameters.get_error_configuration()? {
             diagnostician.with_configuration(configuration)
         } else {
             diagnostician
         };
 
+        let diagnostician = Arc::new(RwLock::new(diagnostician));
+
         // TODO: This can be improved quite a bit, e.g. `GlobalContext::new(project);`, to do that see the
         //       commented `project` method in the GlobalContext implementation block
         let context = GlobalContext::new()
@@ -220,7 +223,7 @@ impl<T: SourceContainer> BuildPipeline<T> {
                 println!("{}", self.project.get_validation_schema().as_ref())
             }
             cli::ConfigOption::Diagnostics => {
-                println!("{}", self.diagnostician.get_diagnostic_configuration())
+                println!("{}", self.diagnostician.read().unwrap().get_diagnostic_configuration())
             }
         };
 
@@ -263,7 +266,7 @@ impl<T: SourceContainer> Pipeline for BuildPipeline<T> {
             &self.compile_parameters
         {
             //Explain the given error
-            println!("{}", self.diagnostician.explain(error));
+            println!("{}", self.diagnostician.read().unwrap().explain(error));
             return Ok(());
         }
 
@@ -281,7 +284,7 @@ impl<T: SourceContainer> Pipeline for BuildPipeline<T> {
 
         // 5. Validate
         //TODO: this goes into a participant
-        annotated_project.validate(&self.context, &mut self.diagnostician)?;
+        annotated_project.validate(&self.context, self.diagnostician.clone())?;
 
         //TODO: probably not needed, should be a participant anyway
         if let Some((location, format)) = self
@@ -303,18 +306,15 @@ impl<T: SourceContainer> Pipeline for BuildPipeline<T> {
     }
 
     fn parse(&mut self) -> Result<ParsedProject, Diagnostic> {
-        let project = ParsedProject::parse(&self.context, &self.project, &mut self.diagnostician)?;
+        let project = ParsedProject::parse(&self.context, &self.project, self.diagnostician.clone())?;
         Ok(project)
     }
 
     fn index(&mut self, project: ParsedProject) -> Result<IndexedProject, Diagnostic> {
         self.participants.iter().for_each(|p| {
-            p.pre_index(&project, &mut self.diagnostician);
+            p.pre_index(&project);
         });
-        let project = self
-            .mutable_participants
-            .iter_mut()
-            .fold(project, |project, p| p.pre_index(project, &mut self.diagnostician));
+        let project = self.mutable_participants.iter_mut().fold(project, |project, p| p.pre_index(project));
         let indexed_project = project.index(self.context.provider());
         self.participants.iter().for_each(|p| {
             p.post_index(&indexed_project);
@@ -435,7 +435,7 @@ impl ParsedProject {
     pub fn parse<T: SourceContainer + Sync>(
         ctxt: &GlobalContext,
         project: &Project<T>,
-        diagnostician: &mut Diagnostician,
+        diagnostician: Arc<RwLock<Diagnostician>>,
     ) -> Result<Self, Diagnostic> {
         //TODO in parallel
         //Parse the source files
@@ -453,7 +453,7 @@ impl ParsedProject {
                     source_code::SourceType::Unknown => unreachable!(),
                 };
 
-                parse_func(source, LinkageType::Internal, ctxt.provider(), diagnostician)
+                parse_func(source, LinkageType::Internal, ctxt.provider(), diagnostician.clone())
             })
             .collect::<Vec<_>>();
 
@@ -465,7 +465,7 @@ impl ParsedProject {
             .iter()
             .map(|it| {
                 let source = ctxt.get(it.get_location_str()).expect("All sources should've been read");
-                parse_file(source, LinkageType::External, ctxt.provider(), diagnostician)
+                parse_file(source, LinkageType::External, ctxt.provider(), diagnostician.clone())
             })
             .collect::<Vec<_>>();
         units.extend(includes);
@@ -477,7 +477,7 @@ impl ParsedProject {
             .flat_map(LibraryInformation::get_includes)
             .map(|it| {
                 let source = ctxt.get(it.get_location_str()).expect("All sources should've been read");
-                parse_file(source, LinkageType::External, ctxt.provider(), diagnostician)
+                parse_file(source, LinkageType::External, ctxt.provider(), diagnostician.clone())
             })
             .collect::<Vec<_>>();
         units.extend(lib_includes);
@@ -606,21 +606,21 @@ impl AnnotatedProject {
     pub fn validate(
         &self,
         ctxt: &GlobalContext,
-        diagnostician: &mut Diagnostician,
+        diagnostician: Arc<RwLock<Diagnostician>>,
     ) -> Result<(), Diagnostic> {
         // perform global validation
         let mut validator = Validator::new(ctxt);
         validator.perform_global_validation(&self.index);
         let diagnostics = validator.diagnostics();
-        let mut severity = diagnostician.handle(&diagnostics);
+        let mut severity = diagnostician.write().unwrap().handle(&diagnostics);
 
         //Perform per unit validation
         self.units.iter().for_each(|AnnotatedUnit { unit, .. }| {
             // validate unit
             validator.visit_unit(&self.annotations, &self.index, unit);
             // log errors
             let diagnostics = validator.diagnostics();
-            severity = severity.max(diagnostician.handle(&diagnostics));
+            severity = severity.max(diagnostician.write().unwrap().handle(&diagnostics));
         });
         if severity == Severity::Error {
             Err(Diagnostic::new("Compilation aborted due to critical errors"))

compiler/plc_driver/src/pipelines/participant.rs

@@ -12,7 +12,7 @@ use std::{
 };
 
 use ast::{
-    ast::{Assignment, AstFactory, AstNode, AstStatement, ReferenceAccess, ReferenceExpr},
+    ast::{Assignment, AstFactory, AstNode, AstStatement, PouType, ReferenceAccess, ReferenceExpr},
     mut_visitor::AstVisitorMut,
     provider::IdProvider,
 };
@@ -23,11 +23,7 @@ use plc::{
     resolver::{AnnotationMap, StatementAnnotation},
     ConfigFormat, OnlineChange, Target,
 };
-use plc_diagnostics::{
-    diagnostician::{self, Diagnostician},
-    diagnostics::Diagnostic,
-};
-use plc_index::GlobalContext;
+use plc_diagnostics::{diagnostician::Diagnostician, diagnostics::Diagnostic};
 use project::{object::Object, project::LibraryInformation};
 use source_code::{source_location::SourceLocation, SourceContainer};
 
@@ -39,7 +35,7 @@ use super::{AnnotatedProject, GeneratedProject, IndexedProject, ParsedProject};
 pub trait PipelineParticipant: Sync + Send {
     /// Implement this to access the project before it gets indexed
     /// This happens directly after parsing
-    fn pre_index(&self, _parsed_project: &ParsedProject, diagnostician: &mut Diagnostician) {}
+    fn pre_index(&self, _parsed_project: &ParsedProject) {}
     /// Implement this to access the project after it got indexed
     /// This happens directly after the index returns
     fn post_index(&self, _indexed_project: &IndexedProject) {}
@@ -74,11 +70,7 @@ pub trait PipelineParticipant: Sync + Send {
 pub trait PipelineParticipantMut {
     /// Implement this to access the project before it gets indexed
     /// This happens directly after parsing
-    fn pre_index(
-        &mut self,
-        parsed_project: ParsedProject,
-        _diagnostician: &mut Diagnostician,
-    ) -> ParsedProject {
+    fn pre_index(&mut self, parsed_project: ParsedProject) -> ParsedProject {
         parsed_project
     }
 
@@ -230,11 +222,7 @@ impl<T: SourceContainer + Send> PipelineParticipant for CodegenParticipant<T> {
 pub struct LoweringParticipant;
 
 impl PipelineParticipantMut for LoweringParticipant {
-    fn pre_index(
-        &mut self,
-        parsed_project: ParsedProject,
-        diagnostician: &mut Diagnostician,
-    ) -> ParsedProject {
+    fn pre_index(&mut self, parsed_project: ParsedProject) -> ParsedProject {
         parsed_project
     }
 
@@ -264,14 +252,16 @@ impl PipelineParticipantMut for InitParticipant {
     }
 }
 
+impl PipelineParticipant for PropertyDesugar {
+    fn pre_index(&self, parsed_project: &ParsedProject) {
+        let ParsedProject { units, .. } = parsed_project;
+        PropertyDesugar::validate_units(&units);
+    }
+}
+
 impl PipelineParticipantMut for PropertyDesugar {
-    fn pre_index(
-        &mut self,
-        parsed_project: ParsedProject,
-        diagnostician: &mut Diagnostician,
-    ) -> ParsedProject {
+    fn pre_index(&mut self, parsed_project: ParsedProject) -> ParsedProject {
         let ParsedProject { mut units, .. } = parsed_project;
-        PropertyDesugar::validate_units(&units);
 
         // desugar
         for unit in &mut units {
@@ -299,9 +289,6 @@ impl PipelineParticipantMut for PropertyDesugar {
                         unreachable!()
                     };
 
-                    // dbg!(annotations.get(&left));
-                    // dbg!(annotations.get(&right));
-
                     if annotations.get(&right).is_some_and(StatementAnnotation::is_property) {
                         insert_get_prefix("get_", right);
 
@@ -322,8 +309,6 @@ impl PipelineParticipantMut for PropertyDesugar {
                             SourceLocation::undefined(),
                         );
 
-                        dbg!(&call);
-
                         std::mem::swap(node, &mut call);
                     }
                 }
@@ -344,36 +329,33 @@ fn insert_get_prefix(prefix: &str, node: &mut AstNode) {
     name.insert_str(0, prefix);
 }
 
-impl PipelineParticipant for PropertyDesugar {
-    fn pre_index(&self, parsed_project: &ParsedProject, diagnostician: &mut Diagnostician) {
-        let ParsedProject { units } = parsed_project;
-
-        for unit in units {
-            for property in &unit.properties {
-                dbg!(&property);
-                if property.implementations.is_empty() {
-                    let diagnostic = Diagnostic::new("test")
-                        .with_location(property.name_location.clone())
-                        .with_error_code("E001");
+pub struct ValidationParticipant {
+    // TODO: global_context at some point
+    diagnostician: Arc<RwLock<Diagnostician>>,
+}
 
-                    dbg!(&diagnostic);
+impl ValidationParticipant {
+    pub fn new(diagnostician: Arc<RwLock<Diagnostician>>) -> ValidationParticipant {
+        ValidationParticipant { diagnostician }
+    }
+}
 
-                    diagnostician.handle(&[diagnostic]);
+impl PipelineParticipant for ValidationParticipant {
+    fn pre_index(&self, parsed_project: &ParsedProject) {
+        let mut diagnostics = Vec::new();
+        for unit in &parsed_project.units {
+            for property in &unit.properties {
+                if !matches!(property.parent_kind, PouType::FunctionBlock | PouType::Program) {
+                    diagnostics.push(
+                        // TODO: Support classes
+                        Diagnostic::new("Property only allowed in FunctionBlock or Program")
+                            .with_location(property.name_location.clone())
+                            .with_error_code("E001"), // TODO: Update me
+                    );
                 }
             }
         }
-    }
-}
 
-pub struct Validator2 {
-    context: Arc<GlobalContext>,
-    diagnostics: Vec<Diagnostic>,
-}
-
-impl Validator2 {
-    pub fn new(context: Arc<GlobalContext>) -> Validator2 {
-        Validator2 { context, diagnostics: Vec::new() }
+        self.diagnostician.write().unwrap().handle(&diagnostics);
     }
 }
-
-impl PipelineParticipantMut for Validator2 {}

compiler/plc_driver/src/runner.rs

@@ -1,3 +1,5 @@
+use std::sync::{Arc, RwLock};
+
 use crate::{pipelines::ParsedProject, CompileOptions};
 
 use plc::codegen::{CodegenContext, GeneratedModule};
@@ -27,8 +29,8 @@ pub fn compile<T: Compilable>(context: &CodegenContext, source: T) -> GeneratedM
     let source = source.containers();
     let project = Project::new("TestProject".to_string()).with_sources(source);
     let ctxt = GlobalContext::new().with_source(project.get_sources(), None).unwrap();
-    let mut diagnostician = Diagnostician::null_diagnostician();
-    let parsed_project = ParsedProject::parse(&ctxt, &project, &mut diagnostician).unwrap();
+    let diagnostician = Arc::new(RwLock::new(Diagnostician::null_diagnostician()));
+    let parsed_project = ParsedProject::parse(&ctxt, &project, diagnostician.clone()).unwrap();
     let indexed_project = parsed_project
         .index(ctxt.provider())
         .extend_with_init_units(&project.get_init_symbol_name(), ctxt.provider());