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

Redefining constant variables does not cause an error #602

Closed
mhasel opened this issue Oct 12, 2022 · 4 comments · Fixed by #622
Closed

Redefining constant variables does not cause an error #602

mhasel opened this issue Oct 12, 2022 · 4 comments · Fixed by #622
Assignees
Labels
bug Something isn't working

Comments

@mhasel
Copy link
Member

mhasel commented Oct 12, 2022

As of now, the following code compiles and validates without error. I'm not sure this is how it should behave. In my opinion, it should not be possible to redefine constants (at the very least not in the same scope they are defined in). Please discuss.

        FUNCTION add : DINT
        VAR CONSTANT
            TWO : DINT := 2;
            THREE : DINT := 3;
        END_VAR
        VAR
            TWO : DINT := 4;
            THREE : DINT := 5;
        END_VAR
            add := TWO + THREE;
        END_FUNCTION
          7 │+define i32 @add() {
          8 │+entry:
          9 │+  %TWO = alloca i32, align 4
         10 │+  %THREE = alloca i32, align 4
         11 │+  %add = alloca i32, align 4
         12 │+  store i32 4, i32* %TWO, align 4
         13 │+  store i32 5, i32* %THREE, align 4
         14 │+  store i32 0, i32* %add, align 4
         15 │+  %load_TWO = load i32, i32* %TWO, align 4
         16 │+  %load_THREE = load i32, i32* %THREE, align 4
         17 │+  %tmpVar = add i32 %load_TWO, %load_THREE
         18 │+  store i32 %tmpVar, i32* %add, align 4
         19 │+  %add_ret = load i32, i32* %add, align 4
         20 │+  ret i32 %add_ret
         21 │+}
@mhasel mhasel added the bug Something isn't working label Oct 12, 2022
@riederm
Copy link
Collaborator

riederm commented Oct 12, 2022

yes this is a problem. qualified names must be unique for the whole application.
Qualified names here are: "add.TWO" and "add.THREE".

This should be solved by allowing the index to store more than one entry per name (use multi-maps instead of hashmap). Every multi-entry is a problem.

With the current implementation the first "add.TWO" will be overwritten in the index by the next "add.TWO"

@riederm
Copy link
Collaborator

riederm commented Oct 12, 2022

related issues:
#354 (duplicate functions should be a problem)
#285 (location information in the index) -> required to report meaningful problems

@mhasel
Copy link
Member Author

mhasel commented Oct 13, 2022

possibly also related, depending on how we want to approach multiple enumerations of the same name.

#569 (Global variable initializer type does not match global variable type)

@ghaith
Copy link
Collaborator

ghaith commented Oct 17, 2022

possibly also related, depending on how we want to approach multiple enumerations of the same name.

#569 (Global variable initializer type does not match global variable type)

I think #569 is not related, in there we need to support multiple enum variants with the same name if they are defined in different enums, the enums can then be accessed using Enum#Variant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants