feat: add typed enums and allow type-specifier to be suffixed #1544

mhasel · 2025-11-10T12:41:14Z

Add support for typed enums with explicit integer type specifications, supporting both IEC 61131-3 standard syntax (TYPE NAME : TYPE (...)) and Codesys syntax (TYPE NAME : (...) TYPE).

Additionally, enums are no longer blanket-zero-initialized but rather they will:

be initialized with an explicit initializer (if present)
be initialized with the 0 variant (if present)
fall back to the first declared variant

Add support for typed enums with explicit integer type specifications, supporting both IEC 61131-3 standard syntax (TYPE NAME : TYPE (...)) and Codesys syntax (TYPE NAME : (...) TYPE).

mhasel · 2025-11-11T09:16:00Z

compiler/plc_driver/src/pipelines.rs

+        let (mut index, unresolvables) = plc::resolver::const_evaluator::evaluate_constants(global_index);
+
+        // Fix up enum defaults after constants are resolved
+        index.finalize_enum_defaults();


Design Question: Should finalize_enum_defaults() be integrated into evaluate_constants()?

Currently, finalize_enum_defaults() is called separately after evaluate_constants() in three places:

Main pipeline (pipelines.rs)

Test utilities (test_utils.rs, 2 locations)

Arguments for keeping them separate (current approach):

Clear separation of concerns: Constant evaluation and enum initialization are conceptually distinct operations

Modularity: Each function has a single, well-defined responsibility

Explicit pipeline: Makes it obvious what post-resolution steps are needed

Arguments for integrating into evaluate_constants():

Strong dependency: finalize_enum_defaults() fundamentally requires resolved constants and cannot work without them

Simpler API: Single function call instead of remembering to call both in sequence

Maintenance: Reduces chance of forgetting this pass in new code paths

Logical cohesion: Both operations finalize constant-related initialization

Should we consider renaming to something like resolve_constants_and_initializers() or updating the evaluate_constants() doc comment to clarify it also handles dependent initialization steps and add this pass to evaluate_constants()?

lets add this to the const_evaluator or a wrapper function calling the the const evaluator and this function, up to you

codecov · 2025-11-11T13:14:22Z

Codecov Report

❌ Patch coverage is 99.42529% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.01%. Comparing base (ce68921) to head (d34959b).
⚠️ Report is 36 commits behind head on master.

Files with missing lines	Patch %	Lines
src/index.rs	96.55%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1544      +/-   ##
==========================================
+ Coverage   94.03%   95.01%   +0.98%     
==========================================
  Files         174      175       +1     
  Lines       58611    56206    -2405     
==========================================
- Hits        55112    53406    -1706     
+ Misses       3499     2800     -699

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

volsa · 2025-11-17T13:22:22Z

src/validation/types.rs

    validate_data_type(validator, data_type, location);

+    // Validate enum numeric type
+    if let DataType::EnumType { numeric_type, .. } = data_type {


Any reason this is not part of the match-statement down below? Maybe you could also create a function for the the validation here? I.e. move down to match statement and call validate_enum_type?

volsa · 2025-11-17T13:24:30Z

src/validation/tests/enum_validation_test.rs

+        TYPE ValidEnum18 : SINT (a := 1, b := 2); END_TYPE
+        TYPE ValidEnum19 : DINT (a := 1, b := 2); END_TYPE
+
+        // Valid: Some integer types (Codesys syntax)


Why just "some" integer types? Can you copy paste cases 11 to 19 and just modify the syntax here?

volsa · 2025-11-17T13:27:08Z

src/validation/tests/enum_validation_test.rs

+
+        PROGRAM main
+            myColor := red;      // Unqualified variant - ok
+            myColor := Color;    // Type itself - should be unresolvable


Whats the value of this? This seems incorrect but there are no diagnostics?

This test is ignored and tracked in #1546

volsa · 2025-11-17T14:39:59Z

compiler/plc_diagnostics/src/diagnostics/error_codes/E122.md

+## Example
+
+```st
+TYPE Color : STRING (red := 1, green := 2, blue := 3);
+END_TYPE
+```
+
+In this example, `STRING` is not a valid base type for an enum. Only integer types are allowed.
+
+## Another example
+
+```st
+TYPE Status : REAL (active := 1, inactive := 0);
+END_TYPE
+
+TYPE Timestamp : TIME (start := 0, stop := 1);
+END_TYPE
+```
+
+These examples show other invalid base types:
+- `REAL` is a floating-point type, not an integer type
+- `TIME` is a time/date type, which although internally represented as an integer, should not be used as an enum base type


Can you just merge the examples instead of having two sections here?

volsa · 2025-11-17T14:43:54Z

src/codegen/tests/typesystem_test.rs

 }

+#[test]
+fn enum_typed_varargs_get_promoted() {


Not sure I understand the purpose of this test? Are we testing whether or not an i16 gets promoted to i32 for the printf?

Yes we are. Otherwise, printf will try to read 32 bits and display garbage unless we explicitly specify a smaller sized int in the format string. We follow the C ABI type promotion rules and therefore enums backed by integer type should do the same

volsa · 2025-11-17T14:55:07Z

src/parser.rs

            let error_range = lexer.source_range_factory.create_range(opening_location..lexer.range().end);

+            // Don't emit warnings if this looks like an enum (will be caught by validation)
+            let is_enum_like = matches!(size_expr.get_stmt(), AstStatement::ExpressionList(_));


Can you give some more context here, e.g. needed because of TYPE InvalidEnum2 : STRING (a := 1, b := 2); END_TYPE

volsa · 2025-11-17T15:52:26Z

src/index.rs

+    /// For each enum without an explicit initializer, this sets the initial_value to:
+    /// 1. The zero-variant (if one exists), or
+    /// 2. The first variant (as fallback)
+    pub fn finalize_enum_defaults(&mut self) {


As discussed offline, maybe add a todo for ghaith to remove this later

volsa · 2025-11-17T15:53:22Z

compiler/plc_driver/src/pipelines.rs

+        let (mut index, unresolvables) = plc::resolver::const_evaluator::evaluate_constants(global_index);
+
+        // Fix up enum defaults after constants are resolved
+        index.finalize_enum_defaults();


lets add this to the const_evaluator or a wrapper function calling the the const evaluator and this function, up to you

mhasel added 3 commits November 10, 2025 13:12

feat: add typed enums

8d97223

Add support for typed enums with explicit integer type specifications, supporting both IEC 61131-3 standard syntax (TYPE NAME : TYPE (...)) and Codesys syntax (TYPE NAME : (...) TYPE).

fmt

2460c83

Merge branch 'master' into mihr/typed-enums

9f534b2

mhasel commented Nov 11, 2025

View reviewed changes

move evaluation to its own pass after constant resolution

b676e54

mhasel force-pushed the mihr/typed-enums branch from 7327160 to b676e54 Compare November 11, 2025 09:22

add issue reference to ignored tests

c27527a

mhasel marked this pull request as ready for review November 11, 2025 09:38

remove some nesting

d34959b

mhasel requested review from ghaith and volsa November 11, 2025 14:22

volsa requested changes Nov 17, 2025

View reviewed changes

feat: add typed enums and allow type-specifier to be suffixed #1544

Are you sure you want to change the base?

feat: add typed enums and allow type-specifier to be suffixed #1544

Uh oh!

Conversation

mhasel commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Design Question: Should finalize_enum_defaults() be integrated into evaluate_constants()?

Arguments for keeping them separate (current approach):

Arguments for integrating into evaluate_constants():

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 11, 2025

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mhasel commented Nov 10, 2025 •

edited

Loading

Design Question: Should `finalize_enum_defaults()` be integrated into `evaluate_constants()`?

Arguments for integrating into `evaluate_constants()`: