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.

Sign up for GitHub

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

Jump to bottom

Duplicate functions should be a problem #354

Closed
riederm opened this issue Oct 29, 2021 · 3 comments · Fixed by #622
Closed

Duplicate functions should be a problem #354

riederm opened this issue Oct 29, 2021 · 3 comments · Fixed by #622
Assignees
@riederm
Labels
bug Something isn't working validation candidate for syntactic or semantic validation

Comments

@riederm
Copy link
Collaborator

riederm commented Oct 29, 2021

If you define a function twice, the compiler does not report a problem.

This is probably also the case for any other POU.
@riederm riederm added bug Something isn't working validation candidate for syntactic or semantic validation labels Oct 29, 2021
@riederm
Copy link
Collaborator Author

riederm commented Jun 1, 2022 •
edited
Loading

internally the index is organized as an IndexMap. This means that we cannot store two functions with the same name.
Should we refactor this to a MultiMap representation so the validator is able to see all (also duplicated) functions?

alternative: the indexer also collects errors and we do not index the 2nd function with the same name (so it will be invisible for the rest of the application's validation). also importing a file's index into the global index may now collect errors.

@ghaith what do you think?

@ghaith
Copy link
Collaborator

ghaith commented Jun 1, 2022

I'm open to having the indexer report errors regardless of the multimap solution.
But for the multimap, what would it bring us more? I can imagine situations where we suggest which function is the more meaningful during validation (Based on params). Still, we're not planning on any function overloading here (or are we🤔), so I don't know if we have immediate benefits..

@ghaith
Copy link
Collaborator

ghaith commented Jun 22, 2022

needs location in index #285

@riederm riederm mentioned this issue Oct 12, 2022
Closed
@riederm riederm self-assigned this Oct 23, 2022
@riederm riederm linked a pull request Oct 29, 2022 that will close this issue
354 duplicate functions should be a problem #622
Merged
@riederm riederm mentioned this issue Nov 3, 2022
Merged
@riederm riederm mentioned this issue Nov 5, 2022
Closed
riederm added a commit that referenced this issue Nov 9, 2022
@riederm
avoid re-registering generic implementations
0abf20f 
when calling a generic function, we register an implementation and
a POU in the index to later link against it. If multiple calls discover
the same generic implementation (e.g. INT-implementation), they were
added mulitple times to the index and the fix implemented in #354 caused
a problem.

ImplementationIndexEntry move back to IndexMap - we only keep one entry
per name - there is no value since the POUIndexEntries handle the
duplication validation.

When importing POUIndexEntries we ignore automatically generated
entries if they are already in the index.
riederm added a commit that referenced this issue Nov 9, 2022
@riederm
avoid re-registering generic implementations
5db99cf 
when calling a generic function, we register an implementation and
a POU in the index to later link against it. If multiple calls discover
the same generic implementation (e.g. INT-implementation), they were
added mulitple times to the index and the fix implemented in #354 caused
a problem.

ImplementationIndexEntry move back to IndexMap - we only keep one entry
per name - there is no value since the POUIndexEntries handle the
duplication validation.

When importing POUIndexEntries we ignore automatically generated
entries if they are already in the index.

fixes #637
@riederm riederm mentioned this issue Nov 9, 2022
Merged
ghaith pushed a commit that referenced this issue Nov 9, 2022
@riederm
avoid re-registering generic implementations (#641)
af83338 
when calling a generic function, we register an implementation and
a POU in the index to later link against it. If multiple calls discover
the same generic implementation (e.g. INT-implementation), they were
added mulitple times to the index and the fix implemented in #354 caused
a problem.

ImplementationIndexEntry move back to IndexMap - we only keep one entry
per name - there is no value since the POUIndexEntries handle the
duplication validation.

When importing POUIndexEntries we ignore automatically generated
entries if they are already in the index.

fixes #637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@riederm riederm

Labels
bug Something isn't working validation candidate for syntactic or semantic validation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

354 duplicate functions should be a problem PLC-lang/rusty
2 participants
@ghaith @riederm