From c5a5c28bd9558522936d914ac1b755bc55d99f9c Mon Sep 17 00:00:00 2001 From: Ghaith Hachem Date: Mon, 21 Feb 2022 09:45:49 +0000 Subject: [PATCH 1/5] Support builtin functions --- Cargo.lock | 1 + Cargo.toml | 1 + src/ast.rs | 1 + src/builtins.rs | 110 ++++++++++++++++++ src/codegen.rs | 2 +- .../generators/expression_generator.rs | 24 ++++ src/codegen/tests/expression_tests.rs | 38 ++++++ ...sion_tests__builtin_function_call_adr.snap | 23 ++++ ...sion_tests__builtin_function_call_ref.snap | 21 ++++ src/index.rs | 14 +++ src/index/tests.rs | 1 + src/index/tests/builtin_tests.rs | 19 +++ src/index/visitor.rs | 12 +- src/lib.rs | 1 + src/resolver.rs | 11 +- src/resolver/tests/resolve_generic_calls.rs | 48 +++++++- src/typesystem.rs | 13 ++- 17 files changed, 331 insertions(+), 9 deletions(-) create mode 100644 src/builtins.rs create mode 100644 src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_adr.snap create mode 100644 src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_ref.snap create mode 100644 src/index/tests/builtin_tests.rs diff --git a/Cargo.lock b/Cargo.lock index ddb55bab86..46afa0d42e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -582,6 +582,7 @@ dependencies = [ "indexmap", "inkwell", "insta", + "lazy_static", "lld_rs", "logos", "num", diff --git a/Cargo.toml b/Cargo.toml index 2ffe95696e..939e434b08 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ regex = "1" serde = { version = "1.0", features = ["derive"] } serde_json = "1" toml = "0.5" +lazy_static = "1.4.0" [dev-dependencies] num = "0.4" diff --git a/src/ast.rs b/src/ast.rs index 67b524a197..15b399b0c0 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -211,6 +211,7 @@ pub struct Implementation { pub enum LinkageType { Internal, External, + BuiltIn, } #[derive(Debug, PartialEq)] diff --git a/src/builtins.rs b/src/builtins.rs new file mode 100644 index 0000000000..55d8ee8e08 --- /dev/null +++ b/src/builtins.rs @@ -0,0 +1,110 @@ +use std::collections::HashMap; + +use inkwell::values::{BasicValue, BasicValueEnum}; +use lazy_static::lazy_static; + +use crate::{ + ast::{AstStatement, CompilationUnit, LinkageType, SourceRange}, + codegen::generators::expression_generator::ExpressionCodeGenerator, + diagnostics::Diagnostic, + lexer::{self, IdProvider}, + parser, +}; + +// Defines a set of functions that are always included in a compiled application +lazy_static! { + static ref BUILTIN: HashMap<&'static str, BuiltIn> = HashMap::from([ + ( + "ADR", + BuiltIn { + decl: "FUNCTION ADR : LWORD + VAR_INPUT + in : T; + END_VAR + END_FUNCTION + ", + code: |generator, params, location| { + if let [reference] = params { + generator + .generate_element_pointer(reference) + .map(|it| generator.ptr_as_value(it)) + } else { + Err(Diagnostic::codegen_error( + "Expected exadtly one parameter for REF", + location, + )) + } + } + }, + ), + ( + "REF", + BuiltIn { + decl: "FUNCTION REF : REF_TO T + VAR_INPUT + in : T; + END_VAR + END_FUNCTION + ", + code: |generator, params, location| { + if let [reference] = params { + generator + .generate_element_pointer(reference) + .map(|it| it.as_basic_value_enum()) + } else { + Err(Diagnostic::codegen_error( + "Expected exadtly one parameter for REF", + location, + )) + } + } + }, + ) + ]); +} + +pub struct BuiltIn { + decl: &'static str, + code: for<'ink, 'b> fn( + &'b ExpressionCodeGenerator<'ink, 'b>, + &[&AstStatement], + SourceRange, + ) -> Result, Diagnostic>, +} + +impl BuiltIn { + pub fn codegen<'ink, 'b>( + &self, + generator: &'b ExpressionCodeGenerator<'ink, 'b>, + params: &[&AstStatement], + location: SourceRange, + ) -> Result, Diagnostic> { + (self.code)(generator, params, location) + } +} + +pub fn parse_built_ins(id_provider: IdProvider) -> (CompilationUnit, Vec) { + let src = BUILTIN + .iter() + .map(|(_, it)| it.decl) + .collect::>() + .join(" "); + parser::parse(lexer::lex_with_ids(&src, id_provider), LinkageType::BuiltIn) +} + +pub fn generate<'ink, 'b>( + builtin: &str, + generator: &'b ExpressionCodeGenerator<'ink, 'b>, + params: Vec<&AstStatement>, + source_location: SourceRange, +) -> Result, Diagnostic> { + BUILTIN + .get(builtin) + .ok_or_else(|| { + Diagnostic::codegen_error( + &format!("Cannot find builtin function {}", builtin), + source_location.clone(), + ) + }) + .and_then(|it| it.codegen(generator, params.as_slice(), source_location)) +} diff --git a/src/codegen.rs b/src/codegen.rs index 4068059d42..54082be406 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -20,7 +20,7 @@ use super::index::*; use inkwell::module::Module; use inkwell::{context::Context, types::BasicType}; -mod generators; +pub(crate) mod generators; mod llvm_index; mod llvm_typesystem; #[cfg(test)] diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs index fb935080ad..59910ff6e2 100644 --- a/src/codegen/generators/expression_generator.rs +++ b/src/codegen/generators/expression_generator.rs @@ -1,6 +1,7 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder use crate::{ ast::{self, DirectAccessType, SourceRange}, + builtins, codegen::llvm_typesystem, diagnostics::{Diagnostic, INTERNAL_LLVM_ERROR}, index::{ImplementationIndexEntry, ImplementationType, Index, VariableIndexEntry}, @@ -449,6 +450,19 @@ impl<'a, 'b> ExpressionCodeGenerator<'a, 'b> { ) })?; + //If the function is builtin, generate a basic value enum for it + if self.index.is_builtin(implementation.get_call_name()) { + return builtins::generate( + implementation.get_call_name(), + self, + parameters + .as_ref() + .map(|it| ast::flatten_expression_list(it)) + .unwrap_or_default(), + operator.get_location(), + ); + } + let (class_ptr, call_ptr) = match implementation { ImplementationIndexEntry { implementation_type: ImplementationType::Function, @@ -914,6 +928,16 @@ impl<'a, 'b> ExpressionCodeGenerator<'a, 'b> { .into_pointer_value() } + pub fn ptr_as_value(&self, ptr: PointerValue<'a>) -> BasicValueEnum<'a> { + let int_type = self.llvm.context.i64_type(); + if ptr.is_const() { + ptr.const_to_int(int_type) + } else { + self.llvm.builder.build_ptr_to_int(ptr, int_type, "") + } + .as_basic_value_enum() + } + /// automatically derefs an inout variable pointer so it can be used like a normal variable /// /// # Arguments diff --git a/src/codegen/tests/expression_tests.rs b/src/codegen/tests/expression_tests.rs index c14ad9dcdf..370a574a52 100644 --- a/src/codegen/tests/expression_tests.rs +++ b/src/codegen/tests/expression_tests.rs @@ -459,3 +459,41 @@ fn nested_call_statements() { // WE expect a flat sequence of calls, no regions and branching insta::assert_snapshot!(result); } + +#[test] +fn builtin_function_call_adr() { + // GIVEN some nested call statements + let result = codegen( + " + PROGRAM main + VAR + a : REF_TO DINT; + b : DINT; + END_VAR + a := ADR(b); + END_PROGRAM + ", + ); + // WHEN compiled + // We expect a direct conversion to lword and subsequent assignment (no call) + insta::assert_snapshot!(result); +} + +#[test] +fn builtin_function_call_ref() { + // GIVEN some nested call statements + let result = codegen( + " + PROGRAM main + VAR + a : REF_TO DINT; + b : DINT; + END_VAR + a := REF(b); + END_PROGRAM + ", + ); + // WHEN compiled + // We expect a direct conversion and subsequent assignment to pointer(no call) + insta::assert_snapshot!(result); +} diff --git a/src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_adr.snap b/src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_adr.snap new file mode 100644 index 0000000000..4f51bccaa4 --- /dev/null +++ b/src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_adr.snap @@ -0,0 +1,23 @@ +--- +source: src/codegen/tests/expression_tests.rs +assertion_line: 479 +expression: result + +--- +; ModuleID = 'main' +source_filename = "main" + +%main_interface = type { i32*, i32 } + +@main_instance = global %main_interface zeroinitializer + +define void @main(%main_interface* %0) { +entry: + %a = getelementptr inbounds %main_interface, %main_interface* %0, i32 0, i32 0 + %b = getelementptr inbounds %main_interface, %main_interface* %0, i32 0, i32 1 + %1 = ptrtoint i32* %b to i64 + %2 = inttoptr i64 %1 to i32* + store i32* %2, i32** %a, align 8 + ret void +} + diff --git a/src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_ref.snap b/src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_ref.snap new file mode 100644 index 0000000000..48a8ed5c38 --- /dev/null +++ b/src/codegen/tests/snapshots/rusty__codegen__tests__expression_tests__builtin_function_call_ref.snap @@ -0,0 +1,21 @@ +--- +source: src/codegen/tests/expression_tests.rs +assertion_line: 499 +expression: result + +--- +; ModuleID = 'main' +source_filename = "main" + +%main_interface = type { i32*, i32 } + +@main_instance = global %main_interface zeroinitializer + +define void @main(%main_interface* %0) { +entry: + %a = getelementptr inbounds %main_interface, %main_interface* %0, i32 0, i32 0 + %b = getelementptr inbounds %main_interface, %main_interface* %0, i32 0, i32 1 + store i32* %b, i32** %a, align 8 + ret void +} + diff --git a/src/index.rs b/src/index.rs index e3b5a461c0..a9c36c1b36 100644 --- a/src/index.rs +++ b/src/index.rs @@ -6,7 +6,9 @@ use crate::{ AstStatement, DirectAccessType, HardwareAccessType, Implementation, LinkageType, PouType, SourceRange, TypeNature, }, + builtins, diagnostics::Diagnostic, + lexer::IdProvider, typesystem::{self, *}, }; @@ -426,6 +428,11 @@ pub struct Index { } impl Index { + pub fn create_with_builtins(id_provider: IdProvider) -> Index { + let (unit, _) = builtins::parse_built_ins(id_provider.clone()); + visitor::visit_index(Index::default(), &unit, id_provider) + } + /// imports all entries from the given index into the current index /// /// imports all global_variables, member_variables, types and implementations @@ -962,6 +969,13 @@ impl Index { ) -> InstanceIterator { InstanceIterator::with_filter(self, inner_filter) } + + pub fn is_builtin(&self, function: &str) -> bool { + //Find a type for that function, see if that type is builtin + self.find_effective_type_info(function) + .map(DataTypeInformation::is_builtin) + .unwrap_or_default() + } } /// Returns a default initialization name for a variable or type diff --git a/src/index/tests.rs b/src/index/tests.rs index ec9c83eb32..d1001bd62b 100644 --- a/src/index/tests.rs +++ b/src/index/tests.rs @@ -1,4 +1,5 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder +mod builtin_tests; mod generic_tests; mod index_tests; mod instance_resolver_tests; diff --git a/src/index/tests/builtin_tests.rs b/src/index/tests/builtin_tests.rs new file mode 100644 index 0000000000..171975e4c3 --- /dev/null +++ b/src/index/tests/builtin_tests.rs @@ -0,0 +1,19 @@ +use crate::{index::Index, lexer::IdProvider, test_utils::tests::index}; + +#[test] +fn builtin_functions_added_to_index() { + let index = Index::create_with_builtins(IdProvider::default()); + assert!(index.find_member("ADR", "in").is_some()); + assert!(index.find_member("REF", "in").is_some()); + assert!(index.find_implementation("ADR").is_some()); + assert!(index.find_implementation("REF").is_some()); +} + +#[test] +fn default_visitor_creates_builtins() { + let (_, index) = index(""); + assert!(index.find_member("ADR", "in").is_some()); + assert!(index.find_member("REF", "in").is_some()); + assert!(index.find_implementation("ADR").is_some()); + assert!(index.find_implementation("REF").is_some()); +} diff --git a/src/index/visitor.rs b/src/index/visitor.rs index a6504e475f..bea4ae50fc 100644 --- a/src/index/visitor.rs +++ b/src/index/visitor.rs @@ -9,9 +9,7 @@ use crate::index::{Index, MemberInfo}; use crate::lexer::IdProvider; use crate::typesystem::{self, *}; -pub fn visit(unit: &CompilationUnit, mut id_provider: IdProvider) -> Index { - let mut index = Index::default(); - +pub fn visit_index(mut index: Index, unit: &CompilationUnit, mut id_provider: IdProvider) -> Index { //Create the typesystem let builtins = get_builtin_types(); for data_type in builtins { @@ -39,6 +37,12 @@ pub fn visit(unit: &CompilationUnit, mut id_provider: IdProvider) -> Index { index } +/// Visits the ast, creating an index of the content. Appends builtin functions to that index +pub fn visit(unit: &CompilationUnit, id_provider: IdProvider) -> Index { + let index = Index::create_with_builtins(id_provider.clone()); + visit_index(index, unit, id_provider) +} + pub fn visit_pou(index: &mut Index, pou: &Pou) { let interface_name = format!("{}_interface", &pou.name); @@ -131,6 +135,7 @@ pub fn visit_pou(index: &mut Index, pou: &Pou) { varargs, source: StructSource::Pou(pou.pou_type.clone()), generics: pou.generics.clone(), + linkage: pou.linkage, }, nature: TypeNature::Any, }; @@ -267,6 +272,7 @@ fn visit_data_type( varargs: None, source: StructSource::OriginalDeclaration, generics: vec![], + linkage: ast::LinkageType::Internal, }; let init = index diff --git a/src/lib.rs b/src/lib.rs index 98fad3ce8e..9b54367d9f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,7 @@ use crate::ast::CompilationUnit; use crate::diagnostics::Diagnostician; use crate::resolver::{AnnotationMapImpl, TypeAnnotator}; mod ast; +mod builtins; pub mod cli; mod codegen; pub mod diagnostics; diff --git a/src/resolver.rs b/src/resolver.rs index 251c3e64c9..334706acce 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -14,7 +14,7 @@ pub mod const_evaluator; use crate::{ ast::{ self, AstId, AstStatement, CompilationUnit, DataType, DataTypeDeclaration, GenericBinding, - Operator, Pou, TypeNature, UserTypeDeclaration, Variable, + LinkageType, Operator, Pou, TypeNature, UserTypeDeclaration, Variable, }, index::{ImplementationIndexEntry, ImplementationType, Index, VariableIndexEntry}, typesystem::{ @@ -1281,8 +1281,11 @@ impl<'i> TypeAnnotator<'i> { .index .get_effective_type_by_name(implementation_name) .get_type_information(); - if let DataTypeInformation::Struct { generics, .. } = operator_type { - if !generics.is_empty() { + if let DataTypeInformation::Struct { + generics, linkage, .. + } = operator_type + { + if linkage != &LinkageType::BuiltIn && !generics.is_empty() { let generic_map = &self.derive_generic_types(generics, generics_candidates); //Annotate the statement with the new function call if let Some(StatementAnnotation::Function { @@ -1333,6 +1336,7 @@ impl<'i> TypeAnnotator<'i> { member_names, source, varargs, + linkage, .. } = &generic_type.get_type_information() { @@ -1343,6 +1347,7 @@ impl<'i> TypeAnnotator<'i> { varargs: varargs.clone(), source: source.clone(), generics: vec![], + linkage: *linkage, }; for member in member_names { if let Some(generic_entry) = self.index.find_member(generic_type.get_name(), member) diff --git a/src/resolver/tests/resolve_generic_calls.rs b/src/resolver/tests/resolve_generic_calls.rs index bfdcfbd31f..17560f1ed8 100644 --- a/src/resolver/tests/resolve_generic_calls.rs +++ b/src/resolver/tests/resolve_generic_calls.rs @@ -3,7 +3,7 @@ use crate::{ ast::{self, AstStatement}, resolver::{AnnotationMap, TypeAnnotator}, test_utils::tests::index, - typesystem::{BYTE_TYPE, DINT_TYPE, INT_TYPE, REAL_TYPE}, + typesystem::{BYTE_TYPE, DINT_TYPE, INT_TYPE, LWORD_TYPE, REAL_TYPE}, }; #[test] @@ -474,3 +474,49 @@ fn call_order_of_generic_parameters_does_not_change_annotations() { } } } + +#[test] +fn builtin_generic_functions_do_not_get_specialized_calls() { + let (unit, index) = index( + " + PROGRAM PRG + VAR + a : INT; + END_VAR + ADR(x := a); + ADR(6); + ADR(1.0); + REF(x := a); + REF(6); + REF(1.0); + END_PROGRAM", + ); + let (annotations, _) = TypeAnnotator::visit_unit(&index, &unit); + //The implementations are not added to the index + let implementations = annotations.new_index.get_implementations(); + assert!(!implementations.contains_key("adr__int")); + assert!(!implementations.contains_key("adr__dint")); + assert!(!implementations.contains_key("adr__real")); + assert!(!implementations.contains_key("ref__int")); + assert!(!implementations.contains_key("ref__dint")); + assert!(!implementations.contains_key("ref__real")); + + //The pous are not added to the index + let pous = annotations.new_index.get_pou_types(); + assert!(!pous.contains_key("adr__int")); + assert!(!pous.contains_key("adr__dint")); + assert!(!pous.contains_key("adr__real")); + assert!(!pous.contains_key("ref__int")); + assert!(!pous.contains_key("ref__dint")); + assert!(!pous.contains_key("ref__real")); + + let call = &unit.implementations[0].statements[0]; + + //The return type should have the correct type + assert_type_and_hint!(&annotations, &index, call, LWORD_TYPE, None); + + let call = &unit.implementations[0].statements[1]; + + //The return type should have the correct type + assert_type_and_hint!(&annotations, &index, call, LWORD_TYPE, None); +} diff --git a/src/typesystem.rs b/src/typesystem.rs index d49931ea95..54a89a25de 100644 --- a/src/typesystem.rs +++ b/src/typesystem.rs @@ -5,7 +5,7 @@ use std::{ }; use crate::{ - ast::{AstStatement, GenericBinding, Operator, PouType, TypeNature}, + ast::{AstStatement, GenericBinding, LinkageType, Operator, PouType, TypeNature}, index::{const_expressions::ConstId, Index}, }; @@ -174,6 +174,7 @@ pub enum DataTypeInformation { varargs: Option, source: StructSource, generics: Vec, + linkage: LinkageType, }, Array { name: TypeId, @@ -375,6 +376,16 @@ impl DataTypeInformation { _ => unimplemented!("Alignment for {}", self.get_name()), } } + + pub fn is_builtin(&self) -> bool { + matches!( + self, + DataTypeInformation::Struct { + linkage: LinkageType::BuiltIn, + .. + } + ) + } } #[derive(Debug, Clone, Copy, PartialEq)] From cfc603fe5632b6419154ad62970581aa2785e56e Mon Sep 17 00:00:00 2001 From: Ghaith Hachem Date: Mon, 21 Feb 2022 10:54:10 +0100 Subject: [PATCH 2/5] Add correctness tests --- src/builtins.rs | 2 +- tests/correctness/pointers.rs | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/builtins.rs b/src/builtins.rs index 55d8ee8e08..dd10996823 100644 --- a/src/builtins.rs +++ b/src/builtins.rs @@ -99,7 +99,7 @@ pub fn generate<'ink, 'b>( source_location: SourceRange, ) -> Result, Diagnostic> { BUILTIN - .get(builtin) + .get(builtin.to_uppercase().as_str()) .ok_or_else(|| { Diagnostic::codegen_error( &format!("Cannot find builtin function {}", builtin), diff --git a/tests/correctness/pointers.rs b/tests/correctness/pointers.rs index bdee4126d5..86c138ecfd 100644 --- a/tests/correctness/pointers.rs +++ b/tests/correctness/pointers.rs @@ -3,6 +3,43 @@ use crate::{compile_and_run, MainType}; mod references; +#[test] +fn pointer_test_builtin() { + let function = r" +TYPE MyStruct: STRUCT x: DINT; y: DINT; END_STRUCT END_TYPE +TYPE MyRef : REF_TO REF_TO DINT; END_TYPE + +FUNCTION main : DINT + main := foo(); +END_FUNCTION + +FUNCTION foo : DINT +VAR + x : DINT; + s : MyStruct; + u,y : REF_TO DINT; + z : REF_TO REF_TO DINT; + v : MyRef; + +END_VAR +u := REF(s.x); +y := u; +z := ADR(y); +s.x := 9; +z^^ := y^*2; +v := z; +y^ := v^^*2; + +foo := y^ + 1; +END_FUNCTION + "; + + let mut maintype = MainType::default(); + + let res: i32 = compile_and_run(function.to_string(), &mut maintype); + + assert_eq!(37, res); +} #[test] fn pointer_test() { let function = r" From f9b9dc455a3538108acfe2bacd41430c3cfbad17 Mon Sep 17 00:00:00 2001 From: Ghaith Hachem Date: Thu, 24 Feb 2022 07:52:54 +0000 Subject: [PATCH 3/5] Review comments The builtinin index is now created first in the lib, and not always by visiting an ast. Also fixed the index import to use not skip enum and hardware constants. As a side effect of the index being imported, some index numbers changed in some tests --- src/builtins.rs | 18 +--- .../generators/expression_generator.rs | 12 +-- src/index.rs | 90 ++++++++++++------- src/index/const_expressions.rs | 29 ++++-- src/index/tests/builtin_tests.rs | 9 +- ...ray_with_const_instances_are_repeated.snap | 12 +-- src/index/visitor.rs | 9 +- src/lib.rs | 5 ++ ...binding__tests__hardware_collected_fb.snap | 30 +++---- src/test_utils.rs | 8 +- 10 files changed, 133 insertions(+), 89 deletions(-) diff --git a/src/builtins.rs b/src/builtins.rs index dd10996823..8a6a5cb736 100644 --- a/src/builtins.rs +++ b/src/builtins.rs @@ -92,19 +92,7 @@ pub fn parse_built_ins(id_provider: IdProvider) -> (CompilationUnit, Vec( - builtin: &str, - generator: &'b ExpressionCodeGenerator<'ink, 'b>, - params: Vec<&AstStatement>, - source_location: SourceRange, -) -> Result, Diagnostic> { - BUILTIN - .get(builtin.to_uppercase().as_str()) - .ok_or_else(|| { - Diagnostic::codegen_error( - &format!("Cannot find builtin function {}", builtin), - source_location.clone(), - ) - }) - .and_then(|it| it.codegen(generator, params.as_slice(), source_location)) +/// Returns the requested functio from the builtin index or None +pub fn get_builtin(name: &str) -> Option<&'static BuiltIn> { + BUILTIN.get(name.to_uppercase().as_str()) } diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs index 59910ff6e2..1a0e30d7e4 100644 --- a/src/codegen/generators/expression_generator.rs +++ b/src/codegen/generators/expression_generator.rs @@ -1,7 +1,6 @@ // Copyright (c) 2020 Ghaith Hachem and Mathias Rieder use crate::{ ast::{self, DirectAccessType, SourceRange}, - builtins, codegen::llvm_typesystem, diagnostics::{Diagnostic, INTERNAL_LLVM_ERROR}, index::{ImplementationIndexEntry, ImplementationType, Index, VariableIndexEntry}, @@ -451,14 +450,17 @@ impl<'a, 'b> ExpressionCodeGenerator<'a, 'b> { })?; //If the function is builtin, generate a basic value enum for it - if self.index.is_builtin(implementation.get_call_name()) { - return builtins::generate( - implementation.get_call_name(), + if let Some(builtin) = self + .index + .get_builtin_function(implementation.get_call_name()) + { + return builtin.codegen( self, parameters .as_ref() .map(|it| ast::flatten_expression_list(it)) - .unwrap_or_default(), + .unwrap_or_default() + .as_slice(), operator.get_location(), ); } diff --git a/src/index.rs b/src/index.rs index a9c36c1b36..587caf333f 100644 --- a/src/index.rs +++ b/src/index.rs @@ -6,9 +6,8 @@ use crate::{ AstStatement, DirectAccessType, HardwareAccessType, Implementation, LinkageType, PouType, SourceRange, TypeNature, }, - builtins, + builtins::{self, BuiltIn}, diagnostics::Diagnostic, - lexer::IdProvider, typesystem::{self, *}, }; @@ -428,11 +427,6 @@ pub struct Index { } impl Index { - pub fn create_with_builtins(id_provider: IdProvider) -> Index { - let (unit, _) = builtins::parse_built_ins(id_provider.clone()); - visitor::visit_index(Index::default(), &unit, id_provider) - } - /// imports all entries from the given index into the current index /// /// imports all global_variables, member_variables, types and implementations @@ -441,43 +435,41 @@ impl Index { /// into the current one pub fn import(&mut self, mut other: Index) { //global variables - for (name, mut e) in other.global_variables.drain(..) { - e.initial_value = - self.maybe_import_const_expr(&mut other.constant_expressions, &e.initial_value); + for (name, e) in other.global_variables.drain(..) { + let e = self.transfer_constants(e, &mut other.constant_expressions); self.global_variables.insert(name, e); } - //enum_global_variables - for (name, mut e) in other.enum_global_variables.drain(..) { - e.initial_value = - self.maybe_import_const_expr(&mut other.constant_expressions, &e.initial_value); - self.enum_global_variables.insert(name, e.clone()); - self.enum_qualified_variables - .insert(e.qualified_name.to_lowercase(), e); + //enmu_variables use the qualified variables since name conflicts will be overriden in the enum_global + for (qualified_name, e) in other.enum_qualified_variables.drain(..) { + let e = self.transfer_constants(e, &mut other.constant_expressions); + dbg!(&e); + self.enum_global_variables + .insert(e.get_name().to_lowercase(), e.clone()); + self.enum_qualified_variables.insert(qualified_name, e); } //initializers - for (name, mut e) in other.global_initializers.drain(..) { - e.initial_value = - self.maybe_import_const_expr(&mut other.constant_expressions, &e.initial_value); + for (name, e) in other.global_initializers.drain(..) { + let e = self.transfer_constants(e, &mut other.constant_expressions); self.global_initializers.insert(name, e); } //member variables for (name, mut members) in other.member_variables.drain(..) { //enum qualified variables - for (_, mut e) in members.iter_mut() { - e.initial_value = - self.maybe_import_const_expr(&mut other.constant_expressions, &e.initial_value); + let mut new_members = IndexMap::default(); + for (name, e) in members.drain(..) { + let e = self.transfer_constants(e, &mut other.constant_expressions); + new_members.insert(name, e); } - self.member_variables.insert(name, members); + self.member_variables.insert(name, new_members); } //types for (name, mut e) in other.type_index.types.drain(..) { e.initial_value = self.maybe_import_const_expr(&mut other.constant_expressions, &e.initial_value); - match &mut e.information { //import constant expressions in array-type-definitions DataTypeInformation::Array { dimensions, .. } => { @@ -511,6 +503,38 @@ impl Index { // self.constant_expressions.import(other.constant_expressions) } + fn transfer_constants( + &mut self, + mut variable: VariableIndexEntry, + import_from: &mut ConstExpressions, + ) -> VariableIndexEntry { + variable.initial_value = self.maybe_import_const_expr(import_from, &variable.initial_value); + + let binding = if let Some(HardwareBinding { + direction, + access, + entries, + location, + }) = variable.get_hardware_binding() + { + let mut new_entries = vec![]; + for entry in entries { + if let Some(e) = self.maybe_import_const_expr(import_from, &Some(*entry)) { + new_entries.push(e); + } + } + Some(HardwareBinding { + direction: *direction, + access: *access, + entries: new_entries, + location: location.clone(), + }) + } else { + None + }; + variable.set_hardware_binding(binding) + } + /// imports the corresponding const-expression (according to the given initializer-id) from the given ConstExpressions /// into self's const-expressions and returns the new Id fn maybe_import_const_expr( @@ -520,7 +544,7 @@ impl Index { ) -> Option { initializer_id .as_ref() - .and_then(|it| import_from.remove(it)) + .and_then(|it| import_from.clone(it)) .map(|(init, target_type, scope)| { self.get_mut_const_expressions() .add_constant_expression(init, target_type, scope) @@ -540,7 +564,7 @@ impl Index { let ts = match type_size { TypeSize::LiteralInteger(_) => Some(*type_size), TypeSize::ConstExpression(id) => import_from - .remove(id) + .clone(id) .map(|(expr, target_type, scope)| { self.get_mut_const_expressions().add_constant_expression( expr, @@ -970,11 +994,17 @@ impl Index { InstanceIterator::with_filter(self, inner_filter) } - pub fn is_builtin(&self, function: &str) -> bool { + /// If the provided name is a builtin function, returns it from the builtin index + pub fn get_builtin_function(&self, name: &str) -> Option<&'_ BuiltIn> { //Find a type for that function, see if that type is builtin - self.find_effective_type_info(function) + if let Some(true) = self + .find_effective_type_info(name) .map(DataTypeInformation::is_builtin) - .unwrap_or_default() + { + builtins::get_builtin(name) + } else { + None + } } } diff --git a/src/index/const_expressions.rs b/src/index/const_expressions.rs index 2e1a5e81f3..07b357ae86 100644 --- a/src/index/const_expressions.rs +++ b/src/index/const_expressions.rs @@ -115,14 +115,29 @@ impl ConstExpressions { self.expressions.get(*id).map(|it| &it.expr) } - /// removes the expression from the ConstExpressions and returns all of its elements - pub fn remove(&mut self, id: &ConstId) -> Option<(AstStatement, String, Option)> { - self.expressions.remove(*id).map(|it| match it.expr { - ConstExpression::Unresolved { statement, scope } => { - (statement, it.target_type_name, scope) + // /// removes the expression from the ConstExpressions and returns all of its elements + // pub fn remove(&mut self, id: &ConstId) -> Option<(AstStatement, String, Option)> { + // self.expressions.remove(*id).map(|it| match it.expr { + // 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), + // }) + // } + + /// clones the expression in the ConstExpressions and returns all of its elements + pub fn clone(&self, id: &ConstId) -> Option<(AstStatement, String, Option)> { + self.expressions.get(*id).map(|it| match &it.expr { + ConstExpression::Unresolved { statement, scope } => ( + statement.clone(), + it.target_type_name.clone(), + scope.clone(), + ), + ConstExpression::Resolved(s) => (s.clone(), it.target_type_name.clone(), None), + ConstExpression::Unresolvable { statement: s, .. } => { + (s.clone(), it.target_type_name.clone(), None) } - ConstExpression::Resolved(s) => (s, it.target_type_name, None), - ConstExpression::Unresolvable { statement: s, .. } => (s, it.target_type_name, None), }) } diff --git a/src/index/tests/builtin_tests.rs b/src/index/tests/builtin_tests.rs index 171975e4c3..73ae3e18e5 100644 --- a/src/index/tests/builtin_tests.rs +++ b/src/index/tests/builtin_tests.rs @@ -1,8 +1,11 @@ -use crate::{index::Index, lexer::IdProvider, test_utils::tests::index}; +use crate::{builtins, lexer::IdProvider, test_utils::tests::index}; #[test] fn builtin_functions_added_to_index() { - let index = Index::create_with_builtins(IdProvider::default()); + let provider = IdProvider::default(); + let (builtins, _) = builtins::parse_built_ins(provider.clone()); + let index = crate::index::visitor::visit(&builtins, provider); + assert!(index.find_member("ADR", "in").is_some()); assert!(index.find_member("REF", "in").is_some()); assert!(index.find_implementation("ADR").is_some()); @@ -10,7 +13,7 @@ fn builtin_functions_added_to_index() { } #[test] -fn default_visitor_creates_builtins() { +fn test_indexer_has_builtins() { let (_, index) = index(""); assert!(index.find_member("ADR", "in").is_some()); assert!(index.find_member("REF", "in").is_some()); diff --git a/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__array_with_const_instances_are_repeated.snap b/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__array_with_const_instances_are_repeated.snap index 58838994e4..eba4668ed4 100644 --- a/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__array_with_const_instances_are_repeated.snap +++ b/src/index/tests/snapshots/rusty__index__tests__instance_resolver_tests__array_with_const_instances_are_repeated.snap @@ -1,6 +1,6 @@ --- source: src/index/tests/instance_resolver_tests.rs -assertion_line: 169 +assertion_line: 170 expression: "index.find_instances().collect::>>()" --- @@ -44,7 +44,7 @@ expression: "index.find_instances().collect::>>()" qualified_name: "MainProg.size", initial_value: Some( Index { - index: 2, + index: 0, generation: 0, }, ), @@ -99,13 +99,13 @@ expression: "index.find_instances().collect::>>()" Dimension { start_offset: ConstExpression( Index { - index: 0, + index: 1, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 1, + index: 2, generation: 0, }, ), @@ -146,13 +146,13 @@ expression: "index.find_instances().collect::>>()" Dimension { start_offset: ConstExpression( Index { - index: 0, + index: 1, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 1, + index: 2, generation: 0, }, ), diff --git a/src/index/visitor.rs b/src/index/visitor.rs index bea4ae50fc..11ef080d3a 100644 --- a/src/index/visitor.rs +++ b/src/index/visitor.rs @@ -9,7 +9,8 @@ use crate::index::{Index, MemberInfo}; use crate::lexer::IdProvider; use crate::typesystem::{self, *}; -pub fn visit_index(mut index: Index, unit: &CompilationUnit, mut id_provider: IdProvider) -> Index { +pub fn visit(unit: &CompilationUnit, mut id_provider: IdProvider) -> Index { + let mut index = Index::default(); //Create the typesystem let builtins = get_builtin_types(); for data_type in builtins { @@ -37,12 +38,6 @@ pub fn visit_index(mut index: Index, unit: &CompilationUnit, mut id_provider: Id index } -/// Visits the ast, creating an index of the content. Appends builtin functions to that index -pub fn visit(unit: &CompilationUnit, id_provider: IdProvider) -> Index { - let index = Index::create_with_builtins(id_provider.clone()); - visit_index(index, unit, id_provider) -} - pub fn visit_pou(index: &mut Index, pou: &Pou) { let interface_name = format!("{}_interface", &pou.name); diff --git a/src/lib.rs b/src/lib.rs index 9b54367d9f..096bb5501b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -440,8 +440,13 @@ fn parse_and_index( linkage: LinkageType, ) -> Result<(Index, Units), Diagnostic> { let mut index = Index::default(); + let mut units = Vec::new(); + //parse the builtins into the index + let (builtins, _) = builtins::parse_built_ins(id_provider.clone()); + index.import(index::visitor::visit(&builtins, id_provider.clone())); + for container in source { let location: String = container.get_location().into(); let e = container diff --git a/src/snapshots/rusty__hardware_binding__tests__hardware_collected_fb.snap b/src/snapshots/rusty__hardware_binding__tests__hardware_collected_fb.snap index d16a9a076c..4c10981319 100644 --- a/src/snapshots/rusty__hardware_binding__tests__hardware_collected_fb.snap +++ b/src/snapshots/rusty__hardware_binding__tests__hardware_collected_fb.snap @@ -1,6 +1,6 @@ --- source: src/hardware_binding.rs -assertion_line: 271 +assertion_line: 256 expression: config --- @@ -132,13 +132,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), @@ -165,13 +165,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), @@ -201,13 +201,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), @@ -237,13 +237,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), @@ -270,13 +270,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), @@ -303,13 +303,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), @@ -338,13 +338,13 @@ expression: config Dimension { start_offset: ConstExpression( Index { - index: 2, + index: 9, generation: 0, }, ), end_offset: ConstExpression( Index { - index: 3, + index: 10, generation: 0, }, ), diff --git a/src/test_utils.rs b/src/test_utils.rs index 2cadf2e511..062b2c2df8 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -6,6 +6,7 @@ pub mod tests { use crate::{ ast::{self, CompilationUnit}, + builtins, diagnostics::{Diagnostic, Diagnostician}, index::{self, Index}, lexer::{self, IdProvider}, @@ -34,12 +35,17 @@ pub mod tests { } fn do_index(src: &str, id_provider: IdProvider) -> (CompilationUnit, Index) { + let mut index = Index::default(); + //Import builtins + let (builtins, _) = builtins::parse_built_ins(id_provider.clone()); + index.import(index::visitor::visit(&builtins, id_provider.clone())); + let (mut unit, ..) = parser::parse( lexer::lex_with_ids(src, id_provider.clone()), ast::LinkageType::Internal, ); ast::pre_process(&mut unit, id_provider.clone()); - let index = index::visitor::visit(&unit, id_provider); + index.import(index::visitor::visit(&unit, id_provider)); (unit, index) } From a4cbbe4a87b8ac2e4e957722cacfd32e6b132138 Mon Sep 17 00:00:00 2001 From: Ghaith Hachem Date: Fri, 4 Mar 2022 07:42:21 +0000 Subject: [PATCH 4/5] Review Comments --- src/builtins.rs | 4 ++-- src/index.rs | 1 - src/index/tests/builtin_tests.rs | 2 +- src/lib.rs | 2 +- src/test_utils.rs | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/builtins.rs b/src/builtins.rs index 8a6a5cb736..934c82ebc9 100644 --- a/src/builtins.rs +++ b/src/builtins.rs @@ -83,13 +83,13 @@ impl BuiltIn { } } -pub fn parse_built_ins(id_provider: IdProvider) -> (CompilationUnit, Vec) { +pub fn parse_built_ins(id_provider: IdProvider) -> CompilationUnit { let src = BUILTIN .iter() .map(|(_, it)| it.decl) .collect::>() .join(" "); - parser::parse(lexer::lex_with_ids(&src, id_provider), LinkageType::BuiltIn) + parser::parse(lexer::lex_with_ids(&src, id_provider), LinkageType::BuiltIn).0 } /// Returns the requested functio from the builtin index or None diff --git a/src/index.rs b/src/index.rs index 587caf333f..38977146ac 100644 --- a/src/index.rs +++ b/src/index.rs @@ -443,7 +443,6 @@ impl Index { //enmu_variables use the qualified variables since name conflicts will be overriden in the enum_global for (qualified_name, e) in other.enum_qualified_variables.drain(..) { let e = self.transfer_constants(e, &mut other.constant_expressions); - dbg!(&e); self.enum_global_variables .insert(e.get_name().to_lowercase(), e.clone()); self.enum_qualified_variables.insert(qualified_name, e); diff --git a/src/index/tests/builtin_tests.rs b/src/index/tests/builtin_tests.rs index 73ae3e18e5..39f400f2b0 100644 --- a/src/index/tests/builtin_tests.rs +++ b/src/index/tests/builtin_tests.rs @@ -3,7 +3,7 @@ use crate::{builtins, lexer::IdProvider, test_utils::tests::index}; #[test] fn builtin_functions_added_to_index() { let provider = IdProvider::default(); - let (builtins, _) = builtins::parse_built_ins(provider.clone()); + let builtins = builtins::parse_built_ins(provider.clone()); let index = crate::index::visitor::visit(&builtins, provider); assert!(index.find_member("ADR", "in").is_some()); diff --git a/src/lib.rs b/src/lib.rs index 096bb5501b..2d45ab593d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -444,7 +444,7 @@ fn parse_and_index( let mut units = Vec::new(); //parse the builtins into the index - let (builtins, _) = builtins::parse_built_ins(id_provider.clone()); + let builtins = builtins::parse_built_ins(id_provider.clone()); index.import(index::visitor::visit(&builtins, id_provider.clone())); for container in source { diff --git a/src/test_utils.rs b/src/test_utils.rs index 062b2c2df8..3d0a6fa008 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -37,7 +37,7 @@ pub mod tests { fn do_index(src: &str, id_provider: IdProvider) -> (CompilationUnit, Index) { let mut index = Index::default(); //Import builtins - let (builtins, _) = builtins::parse_built_ins(id_provider.clone()); + let builtins = builtins::parse_built_ins(id_provider.clone()); index.import(index::visitor::visit(&builtins, id_provider.clone())); let (mut unit, ..) = parser::parse( From 84eddfcb9e7decfbd6792b840b05f264212e8fdb Mon Sep 17 00:00:00 2001 From: Ghaith Hachem Date: Fri, 18 Mar 2022 10:30:40 +0100 Subject: [PATCH 5/5] Run clippy and fmt --- src/codegen/generators/expression_generator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codegen/generators/expression_generator.rs b/src/codegen/generators/expression_generator.rs index 1a0e30d7e4..a115637ac3 100644 --- a/src/codegen/generators/expression_generator.rs +++ b/src/codegen/generators/expression_generator.rs @@ -458,7 +458,7 @@ impl<'a, 'b> ExpressionCodeGenerator<'a, 'b> { self, parameters .as_ref() - .map(|it| ast::flatten_expression_list(it)) + .map(ast::flatten_expression_list) .unwrap_or_default() .as_slice(), operator.get_location(),