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

The NF should catch uninitialized variables in functions #9717

Closed
casella opened this issue Nov 17, 2022 · 3 comments · Fixed by #9761
Closed

The NF should catch uninitialized variables in functions #9717

casella opened this issue Nov 17, 2022 · 3 comments · Fixed by #9761
Assignees
Labels
COMP/OMC/Frontend Issue and pull request related to the frontend enhancement
Milestone

Comments

@casella
Copy link
Contributor

casella commented Nov 17, 2022

The old frontend was able to catch uninitialized variables in functions. For example, compiling this MWE with the OF

model TestUninitialized
  Real y = f(time);
  function f
    input Real x;
    output Real y;
  protected
    Real z;
  algorithm
     y := z + x + y;
     y := y + 1;
  end f;
end TestUninitialized;

gave the following warnings:

[1] 11:37:58 Translation Warning
[TestUninitialized: 9:6-9:20]: [z was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.](omeditmessagesbrowser:///TestUninitialized?lineNumber=9)

[2] 11:37:58 Translation Warning
[TestUninitialized: 9:6-9:20]: [y was used before it was defined (given a value). Additional such uses may exist for the variable, but some messages were suppressed.](omeditmessagesbrowser:///TestUninitialized?lineNumber=9)

This should be ported to the NF as well.

@casella casella added enhancement COMP/OMC/Frontend Issue and pull request related to the frontend labels Nov 17, 2022
perost added a commit to perost/OpenModelica that referenced this issue Nov 25, 2022
perost added a commit to perost/OpenModelica that referenced this issue Nov 25, 2022
perost added a commit to perost/OpenModelica that referenced this issue Nov 25, 2022
perost added a commit that referenced this issue Nov 25, 2022
@perost
Copy link
Member

perost commented Nov 25, 2022

#9761 adds a check for using uninitialized values in functions, it's not perfect but should be approximately the same as in the OF. #9763 additionally adds a check for uninitialized outputs since the algorithm returns those anyway.

It's a bit inconsistent because the first is just a warning while the second is an error, because that's how they were already defined. Perhaps both should be an error. The specification does say that using an uninitialized variable is an error anyway, and the algorithm is very conservative and shouldn't have any false positives.

@casella
Copy link
Contributor Author

casella commented Nov 27, 2022

If you are sure that false positives are unlikely, I would make it an error. But we have to be 100% certain about that.

@casella casella added this to the 1.21.0 milestone Nov 27, 2022
@perost
Copy link
Member

perost commented Nov 27, 2022

If you are sure that false positives are unlikely, I would make it an error. But we have to be 100% certain about that.

It seems we do get one false positive in the library testing, one of the ThermoPower models guards a function from being called with an if-expression. So if we want to allow such functions as long as they're not actually called I guess we need to change it to be a warning instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP/OMC/Frontend Issue and pull request related to the frontend enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants