Skip to content

Fix false-positive warning 240 messages #582

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

Merged
merged 2 commits into from
Oct 20, 2020
Merged

Conversation

Daniel-Cortez
Copy link
Contributor

@Daniel-Cortez Daniel-Cortez commented Oct 2, 2020

What this PR does / why we need it:

Fixes false-positive warning 240: previously assigned value is never used messages printed for local variables in (rather rare) cases when a function first assigns a value to a static variable, then calls itself recursively and uses the assigned value in the nested call.
Example:

RecursiveFunc()
{
    static n = 0;
    if (n == 0) // assignment "n = 1" is used here in a nested call
    {
        n = 1;
        RecursiveFunc();
        n = 0; // warning 240: previously assigned value is never used (symbol "n")
    }
}

There's no way to analyze such complicated uses of static local variables accurately because the compiler works in a linear way, so as a solution I disabled tracking of unused assigned values for static variables completely.

Which issue(s) this PR fixes:

Fixes #

What kind of pull this is:

  • A Bug Fix
  • A New Feature
  • Some repository meta (documentation, etc)
  • Other

Additional Documentation:

@Daniel-Cortez Daniel-Cortez requested a review from a team as a code owner October 2, 2020 16:49
@Daniel-Cortez Daniel-Cortez deleted the w240 branch October 19, 2020 14:07
@Daniel-Cortez Daniel-Cortez restored the w240 branch October 19, 2020 14:12
@Daniel-Cortez Daniel-Cortez reopened this Oct 19, 2020
@Daniel-Cortez
Copy link
Contributor Author

Daniel-Cortez commented Oct 19, 2020

Accidentally removed the branch for this PR (and by doing that closed the PR itself) when cleaning up the old branches, because I already used the same branch for #480 and #533. Should have used a different branch for each PR, I guess 😅
Anyway, the PR has been restored and there should be no changes from the moment it was approved.

@Y-Less
Copy link
Member

Y-Less commented Oct 20, 2020

So there's actually another bug with static locals, even when not recursive:

https://github.com/pawn-lang/YSI-Includes/blob/5.x/YSI_Visual/y_commands/y_commands_impl.inc#L463

That line gets a warning for sRet, despite the fact that it might be used here:

https://github.com/pawn-lang/YSI-Includes/blob/5.x/YSI_Visual/y_commands/y_commands_impl.inc#L485

This fix does fix that warning, but it's not really the same thing and "fix" is a bit of a stretch, so I'm reporting it anyway.

@Y-Less Y-Less merged commit 7ea9934 into pawn-lang:dev Oct 20, 2020
@Daniel-Cortez Daniel-Cortez deleted the w240 branch October 22, 2020 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants