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

Another warning 240 false positive. #669

Open
Y-Less opened this issue Aug 8, 2021 · 7 comments
Open

Another warning 240 false positive. #669

Y-Less opened this issue Aug 8, 2021 · 7 comments

Comments

@Y-Less
Copy link
Member

Y-Less commented Aug 8, 2021

Issue description:

@Daniel-Cortez

https://github.com/pawn-lang/YSI-Includes/blob/cb8a4cd99776b99b0c07933d0be29828c2199a8b/YSI_Storage/y_ini/y_ini_impl.inc#L990

The compiler reports this instance of end as never read, but it might be, depending on what is executed in the loop. It is entirely possible to exit the loop without reassigning end.

There's also a different false positive here:

https://github.com/pawn-lang/YSI-Includes/blob/cb8a4cd99776b99b0c07933d0be29828c2199a8b/YSI_Storage/y_ini/y_ini_impl.inc#L1427

handle is assigned at the end of the case statement, but again this is within a loop and it can be used on later iterations.

Minimal complete verifiable example (MCVE):

Workspace Information:

  • Compiler version:
  • Command line arguments provided (or sampctl version):
  • Operating System:
@Y-Less
Copy link
Member Author

Y-Less commented Aug 8, 2021

	#pragma unused end

Doesn't seem to bypass the warning. I think it should.

@Y-Less Y-Less closed this as completed Aug 8, 2021
@Y-Less Y-Less reopened this Aug 8, 2021
@Y-Less
Copy link
Member Author

Y-Less commented Aug 8, 2021

OK, I think the first one for end is not applicable, turns out it was right - it was referring to the clobbered write there. The second is still true though.

@Daniel-Cortez
Copy link
Contributor

Daniel-Cortez commented Aug 9, 2021

	#pragma unused end

Doesn't seem to bypass the warning. I think it should.

Of course it wouldn't, you're using it when the warning is already printed. It should work if you place it before the assignment though.

And regarding that false-positive, I'll look into it

@Daniel-Cortez
Copy link
Contributor

MCVE:

main()
{
    for (new x = 5; --x != 0;)
    {
        if (x == 2)
        {
            x = 1;
            goto lbl_cont;
        }
        x = 3; // false-positive warning 240
    lbl_cont:
    }
}

Replacing goto lbl_cont; with continue; and removing the label declaration seems to fix the warning, so I think this might be related to how goto works.

@Y-Less
Copy link
Member Author

Y-Less commented Aug 10, 2021

Putting it before makes sense. But maybe the message could be improved a bit, because I merged the PR and keep getting caught out. The problem is:

h = 5;
h = 5; // Warning is here.

The warning reads like that second value is the one written and never read, whereas the warning comes when the unread value is clobbered. I understand why, it's just a bit confusing right now.

@Daniel-Cortez
Copy link
Contributor

Daniel-Cortez commented Aug 23, 2021

If I do that, then, for example, if we have this code:

main()
{
	new var1 = 1; // warning 240: assigned value is never used
	new var2 = sizeof(var1); // warning 223: redundant "sizeof": argument size is always 1
	var1 = 2;
	#pragma unused var1, var2
}

the compiler would print warning 240 after warning 223, although the unused assignment happens before the redundant use of sizeof.
This is why initially I made it point at the line of the assignment next to the unused one, but now that I think about it, this is indeed counter-intuitive, and having the compiler point at the exact line of the unused assignment might be more convenient, even if the line numbers of multiple warnings come out of order as a result (also, warnings 203 and 204 already work that way, so this isn't something new). Such change might require some ugly workarounds (the line number for the next assignment is stored into sym->lnumber before the variable is checked for the previously assigned value being used, thus clobbering the line number of the previous assignment) but I'll see what I can do about it.

@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the state: stale label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants