Support for constants issue208 #279
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #279 +/- ##
==========================================
- Coverage 95.27% 94.83% -0.44%
==========================================
Files 44 45 +1
Lines 12051 12784 +733
==========================================
+ Hits 11481 12124 +643
- Misses 570 660 +90
Continue to review full report at Codecov.
|
ghaith
reviewed
Sep 9, 2021
src/lexer/tokens.rs
Outdated
| @@ -12,126 +12,126 @@ pub enum Token { | |||
| #[token("@EXTERNAL")] | |||
| PropertyExternal, | |||
|
|
|||
| #[token("PROGRAM", ignore(case))] | |||
| #[token("PROGRAM")] | |||
There was a problem hiding this comment.
Could you revert this?
I suggest we add the lexer to a separate crate later to avoid the issues
riederm
force-pushed
the
support_for_constants_issue208
branch
from
dc7c09e to
6373018
Compare
const expressions are no longer stored directly in the index but they are stored in an arena. This way it is easier to replace them later when we want to resolve the expressions into literals.
the index now offers a arena for const rexpressions so they
can be later evaluated and replaced (with the same id)
so validation and codegen can use numbers that are
available at compile-time
const expressions include:
- intializers x : INT := LEN - 1
-------
- array-dimensions x : ARRAY[0..LEN-1] OF INT;
- -----
- String-length x : STRING(LEN-1);
-----
the TypeSize enum can either represent a const-expression that is meant to be evaluated at compile time (e.g. LEN - 1) or a compile time literal (e.g. the default length of a String). adresses review commentss
literals and refrences to ints and bools
special handling for unsigned numbers!
Dimensions keep the ast-statements as start and end which eventually will be replaced by a LiteralInteger if it can be evaluated. this allows to collect types and constants in the visit-phase and following that, replace the dimensions that consist of complex expressoins (e.g. "a + b") with a literal-integer once all constants are known.
riederm
force-pushed
the
support_for_constants_issue208
branch
from
4eb3ce6 to
2a35b70
Compare
riederm
force-pushed
the
support_for_constants_issue208
branch
from
e970f97 to
6241639
Compare
improve test coverage qualified references for now can be of the form: POU.variable referencing fields of const structs (E.g. a.b.c) are not supported yet
|
introduces parts of #208 |
ghaith
requested changes
Sep 21, 2021
There was a problem hiding this comment.
Looks good, but I think one big topic we are missing is using constants in the implementation
What happens to a load expression for that variable? Will it behave like a normal reference? Does this reference exist? Could you add some codegen and correctness tests where constants have been used? What happens if we try to write to a constant value? I didn't see a test for that, but maybe I missed it?
src/lexer/tests/lexer_tests.rs
Outdated
| @@ -661,6 +661,7 @@ fn multi_named_keywords_without_underscore_test() { | |||
| assert_eq!(d2.get_location(), SourceRange::new(191..200)); | |||
| } | |||
|
|
|||
| #[ignore = "reason"] | |||
tests that shows that constants can be used in body the current implementation creates global variables for constants and references these variables in expressions. global's could be inlined to create more efficient code see issue #291
riederm
force-pushed
the
support_for_constants_issue208
branch
from
86182d8 to
024c767
Compare
ghaith
previously approved these changes
Sep 23, 2021
riederm
force-pushed
the
support_for_constants_issue208
branch
2 times, most recently
from
582ca8e to
2a03ed4
Compare
…_for_constants_issue208
riederm
force-pushed
the
support_for_constants_issue208
branch
from
2a03ed4 to
1909771
Compare
|
@ghaith can you re-approve plz |
ghaith
approved these changes
Sep 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.