Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Properties #1396

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e972903
lexer: Property related keywords
volsa Jan 24, 2025
3680e52
parser: Parse properties inside POUs
volsa Jan 25, 2025
84bad5e
lowering: Lower properties after parsing
volsa Jan 25, 2025
aa8eb6b
lowering: Lower property references to get or set calls
volsa Jan 26, 2025
5f12e96
add validation and basic tests for it
abroooo Jan 27, 2025
3ab053d
tests: Add and update parser / lowering tests
volsa Jan 27, 2025
8c34910
add lowering tests - not working yet
abroooo Jan 28, 2025
62184e4
test: Introduce lit test
volsa Jan 28, 2025
d31c721
Merge branch 'master' of https://github.com/plc-lang/rusty into volsa…
volsa Jan 29, 2025
27dcee1
Make PropertyLowerer and Validator default participants
volsa Jan 29, 2025
eca00f0
lit: More tests
volsa Jan 29, 2025
2844a2c
lit: Some more tests
volsa Jan 29, 2025
1feafe2
lit: add more complex test
abroooo Jan 29, 2025
4c1aadc
validation: Add temporary(?) validation
volsa Jan 30, 2025
ddcfaed
refactor: Update error codes, add description
volsa Jan 30, 2025
77c1a37
refactor: polish code
abroooo Jan 30, 2025
dae9b16
poc: GlobalContext Error Handling
volsa Jan 30, 2025
6fadfb2
add docstring
abroooo Jan 31, 2025
80c8c3d
fix: Resolver unit test using two different ID providers
volsa Jan 31, 2025
1889930
Merge branch 'volsa/property2' of github.com:PLC-lang/rusty into vols…
volsa Jan 31, 2025
19977cb
refactor: Polish code, part I
volsa Jan 31, 2025
50a25f7
doc: improve docstring
abroooo Jan 31, 2025
3386fe4
refactor: clarify naming
abroooo Jan 31, 2025
6b4deca
fix: GET/SET and END_GET/END_SET must match
abroooo Jan 31, 2025
42d6c86
lit: make recursion test failable
volsa Feb 3, 2025
792f557
chore: add some further todos
volsa Feb 3, 2025
d420b50
test: remove keyword from code
volsa Feb 3, 2025
f9c4a4d
refactor: Polish code, part 2
volsa Feb 3, 2025
c9e8fa7
test: add test for checking for correct endblocks
abroooo Feb 4, 2025
d3ab667
docs: Update and add missing docs
volsa Feb 4, 2025
904fac9
tests: Fix snapshots
volsa Feb 7, 2025
5387fde
Make clippy happy, add TODO for validator location
volsa Feb 7, 2025
dceeec6
Merge branch 'master' into volsa/property2
volsa Feb 7, 2025
5be9a8c
test: Update assert statements
volsa Feb 11, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file removed .swp
Binary file not shown.
19 changes: 18 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

75 changes: 68 additions & 7 deletions compiler/plc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,40 @@
pub location: SourceLocation,
}

#[derive(Debug, PartialEq, Clone)]
pub struct Property {
pub name: String,
pub name_location: SourceLocation,
pub parent_kind: PouType,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to track the PouType here or can we get it via name from the index?

pub parent_name: String,
pub parent_name_location: SourceLocation,
pub datatype: DataTypeDeclaration,
pub implementations: Vec<PropertyImplementation>,
}

#[derive(Debug, PartialEq, Clone)]
pub struct PropertyImplementation {
pub kind: PropertyKind,
pub location: SourceLocation,
pub variable_blocks: Vec<VariableBlock>,
pub body: Vec<AstNode>,
}

#[derive(Debug, PartialEq, Clone, Copy)]
pub enum PropertyKind {
Get,
Set,
}

impl std::fmt::Display for PropertyKind {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
PropertyKind::Get => write!(f, "get"),
PropertyKind::Set => write!(f, "set"),
}
}
}

#[derive(Debug, PartialEq, Eq)]
pub enum PolymorphismMode {
None,
Expand Down Expand Up @@ -269,7 +303,7 @@
BuiltIn,
}

#[derive(Debug, PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum AccessModifier {
Private,
Public,
Expand All @@ -287,6 +321,9 @@
Method {
/// The parent of this method, i.e. a function block, class or an interface
parent: String,

/// The fully qualified name of the property this GET or SET method represents
property: Option<String>,
},
Init,
ProjectInit,
Expand All @@ -310,7 +347,7 @@
impl PouType {
/// returns Some(owner_class) if this is a `Method` or otherwhise `None`
pub fn get_optional_owner_class(&self) -> Option<String> {
if let PouType::Method { parent } = self {
if let PouType::Method { parent, .. } = self {
Some(parent.clone())
} else {
None
Expand All @@ -320,6 +357,10 @@
pub fn is_function_method_or_init(&self) -> bool {
matches!(self, PouType::Function | PouType::Init | PouType::ProjectInit | PouType::Method { .. })
}

pub fn is_stateful(&self) -> bool {
matches!(self, PouType::FunctionBlock | PouType::Program | PouType::Class)
}
}

#[derive(Debug, PartialEq, Clone)]
Expand Down Expand Up @@ -352,6 +393,7 @@
pub interfaces: Vec<Interface>,
pub user_types: Vec<UserTypeDeclaration>,
pub file_name: String,
pub properties: Vec<Property>,
}

impl CompilationUnit {
Expand All @@ -364,6 +406,7 @@
interfaces: Vec::new(),
user_types: Vec::new(),
file_name: file_name.to_string(),
properties: Vec::new(),
}
}

Expand Down Expand Up @@ -395,6 +438,9 @@
Global,
InOut,
External,

/// A compiler internal variable block representing all properties defined within a stateful POU
Property,
}

impl Display for VariableBlockType {
Expand All @@ -407,6 +453,7 @@
VariableBlockType::Global => write!(f, "Global"),
VariableBlockType::InOut => write!(f, "InOut"),
VariableBlockType::External => write!(f, "External"),
VariableBlockType::Property => write!(f, "Property"),

Check warning on line 456 in compiler/plc_ast/src/ast.rs

View check run for this annotation

Codecov / codecov/patch

compiler/plc_ast/src/ast.rs#L456

Added line #L456 was not covered by tests
}
}
}
Expand All @@ -417,7 +464,7 @@
ByRef,
}

#[derive(PartialEq)]
#[derive(PartialEq, Clone)]
pub struct VariableBlock {
pub access: AccessModifier,
pub constant: bool,
Expand All @@ -438,6 +485,19 @@
self.variables = variables;
self
}

/// Creates a new (internal) variable block with a block type of [`Property`]
pub fn property(variables: Vec<Variable>) -> VariableBlock {
VariableBlock {
access: AccessModifier::Internal,
constant: false,
retain: false,
variables,
variable_block_type: VariableBlockType::Property,
linkage: LinkageType::Internal,
location: SourceLocation::internal(),
}
}
}

impl Default for VariableBlock {
Expand Down Expand Up @@ -1300,7 +1360,7 @@
assert_eq!(PouType::FunctionBlock.to_string(), "FunctionBlock");
assert_eq!(PouType::Action.to_string(), "Action");
assert_eq!(PouType::Class.to_string(), "Class");
assert_eq!(PouType::Method { parent: "...".to_string() }.to_string(), "Method");
assert_eq!(PouType::Method { parent: String::new(), property: None }.to_string(), "Method");
}

#[test]
Expand Down Expand Up @@ -1496,11 +1556,12 @@
}

/// creates a new Identifier
pub fn create_identifier<T>(name: &str, location: T, id: AstId) -> AstNode
pub fn create_identifier<T, U>(name: T, location: U, id: AstId) -> AstNode
where
T: Into<SourceLocation>,
T: Into<String>,
U: Into<SourceLocation>,
{
AstNode::new(AstStatement::Identifier(name.to_string()), id, location.into())
AstNode::new(AstStatement::Identifier(name.into()), id, location.into())
}

pub fn create_unary_expression(
Expand Down
12 changes: 6 additions & 6 deletions compiler/plc_diagnostics/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,17 @@ pub enum Severity {
#[derive(Debug)]
pub struct Diagnostic {
/// The Description of the error being reported.
message: String,
pub message: String,
/// Primary location where the diagnostic occurred
primary_location: SourceLocation,
pub primary_location: SourceLocation,
/// Seconday locations relevant to the diagnostics
secondary_locations: Option<Vec<SourceLocation>>,
pub secondary_locations: Option<Vec<SourceLocation>>,
/// Error code for reference in the documentation
error_code: &'static str,
pub error_code: &'static str,
/// Children of the current diagnostic
sub_diagnostics: Vec<Diagnostic>,
pub sub_diagnostics: Vec<Diagnostic>,
/// If the diagnostic is caused by an error, this field contains the original error
internal_error: Option<anyhow::Error>,
pub internal_error: Option<anyhow::Error>,
}

impl std::error::Error for Diagnostic {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ lazy_static! {
E087, Error, include_str!("./error_codes/E087.md"),
E088, Error, include_str!("./error_codes/E088.md"),
E089, Error, include_str!("./error_codes/E089.md"),
E090, Warning, include_str!("./error_codes/E090.md"), // Incompatible reference Assignment
E090, Warning, include_str!("./error_codes/E090.md"), // Incompatible reference Assignment
E091, Warning, include_str!("./error_codes/E091.md"),
E092, Info, include_str!("./error_codes/E092.md"),
E093, Warning, include_str!("./error_codes/E093.md"),
Expand All @@ -215,6 +215,9 @@ lazy_static! {
E111, Error, include_str!("./error_codes/E111.md"), // Duplicate interface methods with different signatures
E112, Error, include_str!("./error_codes/E112.md"), // Incomplete interface implementation
E113, Warning, include_str!("./error_codes/E113.md"), // Interface default method implementation
E114, Error, include_str!("./error_codes/E114.md"), // Property in unsupported POU type
E115, Error, include_str!("./error_codes/E115.md"), // Property defined in non-supported variable block
Comment on lines +218 to +219
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsupported/non-supported - should we go with "unsupported" for both?

E116, Error, include_str!("./error_codes/E116.md"), // Property with invalid number of GET and/or SET blocks
);
}

Expand Down
15 changes: 15 additions & 0 deletions compiler/plc_diagnostics/src/diagnostics/error_codes/E114.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Property defined in non-stateful POU type

Properties may only be defined in stateful POUs such as a `PROGRAM`,`CLASS` or `FUNCTION_BLOCK`.

Errouneus code example:
```
FUNCTION foo
// Invalid definition
PROPERTY bar : DINT
GET
bar := 42;
END_GET
END_PROPERTY
END_FUNCTION
```
15 changes: 15 additions & 0 deletions compiler/plc_diagnostics/src/diagnostics/error_codes/E115.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Property defined in non-supported variable block
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be unsupported?


Properties only allow for variable blocks of type `VAR`.

Errouneus code example:
```
FUNCTION foo
PROPERTY bar : DINT
GET
VAR /* ... */ END_VAR
VAR_INPUT /* ... */ END_VAR // Invalid
END_GET
END_PROPERTY
END_FUNCTION
```
24 changes: 24 additions & 0 deletions compiler/plc_diagnostics/src/diagnostics/error_codes/E116.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Property with invalid number of GET and/or SET blocks

Properties must be non empty and at most contain one block of type GET or SET.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we emphasis (bold and/or italic) at most?


Errouneus code example:
```
FUNCTION foo
PROPERTY bar : DINT
// Invalid, empty (no GET or SET block)
END_PROPERTY
PROPERTY baz : DINT
// Invalid, two GET blocks
GET END_GET
GET END_GET
END_PROPERTY
PROPERTY qux : DINT
// Invalid, two SET blocks
SET END_SET
SET END_SET
END_PROPERTY
END_FUNCTION
```
Loading
Loading