pawn-lang / compiler

Pawn compiler for SA-MP with bug fixes and new features - runs on Windows, Linux, macOS
Other
307 stars 72 forks source link

Compiler giving false positive warnings (240) #717

Closed xunder-matth closed 2 years ago

xunder-matth commented 2 years ago

Issue description:

3.10.11 compiler gives false positive warnings (240: previously assigned value is never used (symbol "i")) when using syntax like one in MVCE.

Minimal complete verifiable example (MCVE):

#include <console>
#include <string>

main() {
    new i;

    if ((i = 1) == 1|| (i = 2) == 2) { // warning 240: previously assigned value is never used (symbol "i")
        printf("%d", i);
    }
}

Workspace Information:

Daniel-Cortez commented 2 years ago

In (i = 1) == 1 you might expect variable i being read before executing operator ==, but it's not.

Assignments in Pawn (and many other C-like languages) not only store the assigned value into a variable, but also yield it as a result of the assignment operation, so in (i = 1) == 1 operand 1 (the first one) also serves as a result of sub-expression (i = 1) - the compiler doesn't generate any code for reading from variable i at all, it just reuses the assigned value. Why read from the variable if you already have the needed value at hand?

As a result, when you assign another value to i (i = 2), that warning 240 message is actually correct.

TL;DR: This is not a bug, this is how assignments in C-like languages work.

xunder-matth commented 2 years ago

If I understand you correctly - you are expecting this code not to work:

#include < console >

main() {
    new i;

    if ((i = 5) == 2 || (i = 3) == 3 || (i = 4) == 4) { // 2x warning 240
        printf("OK! %d", i); // Output: OK! 3
    }
}

More practical example of this:

#include < console >
#include < string >

main() {
    new i, string[] = "S,o.m!e?thing";

    while((i = strfind(string, "!")) != -1 || (i = strfind(string, ",")) != -1 || (i = strfind(string, ".")) != -1 || (i = strfind(string, "?")) != -1) {
        printf("Char %c (idx %d) must be deleted", string[i], i);
        strdel(string, i, i + 1); // Avoid infinite loop
    }
}

This code will work but again compiler will output 3 warnings 240.

Daniel-Cortez commented 2 years ago

If I understand you correctly - you are expecting this code not to work

No, I never said your code isn't supposed to work.

More practical example of this:

...

This code will work but again compiler will output 3 warnings 240.

Yes, because variable i gets written to up to 4 times (the actual number of writes depends on the contents of the input text), but never gets read until it's used in printf - again, not because your code doesn't work, but because of the way how assignments work as sub-expressions (you can re-read my previous post for a more thorough explanation).

Although I do agree that having warning 240 in such situations might be undesirable, I'm not sure how to reasonably "fix" this without sacrificing potential for detecting actual mistakes. In other words, what you have here is simply a rare corner-case, and I don't think there's a way to make the compiler distinguish it from other situations when there's a real mistake made by the user.

In any case, for now you can either

  1. use #pragma warning push/disable 240/pop to temporarily disable the diagnostic, or
  2. simply add extra code to actually read from the variable before it gets assigned the next value:
    while((i = strfind(string, "!"), i != -1) || (i = strfind(string, ","), i != -1) || (i = strfind(string, "."), i != -1) || (i = strfind(string, "?")) != -1) {

    but then you'll end up with a slightly slower compiled code, so I'd recommend using option 1.

xunder-matth commented 2 years ago

Although I do agree that having warning 240 in such situations might be undesirable, I'm not sure how to reasonably "fix" this without sacrificing potential for detecting actual mistakes. In other words, what you have here is simply a rare corner-case, and I don't think there's a way to make the compiler distinguish it from other situations when there's a real mistake made by the user.

Understandable.

Thanks.