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

Eliminate incorrect warning for useless building #4899

Conversation

bjorng
Copy link
Contributor

@bjorng bjorng commented May 27, 2021

In OTP 24, there will be a warning for the following code:

foo(X) ->
    Y = ".",
    fun() ->
            X = Y ++ "."  %Warning: a term is constructed, but never used
    end(),
    ok.

The warning is incorrect, because the built term will be matched to
the previous value of X. This commit eliminates the warning.

To explain the bug, I will illustrate how the example program is transformed
by the Core Erlang passes. My illustrative examples will use Erlang instead
of Core Erlang.

The first transformation makes the match X = Y ++ "." explicit:

foo(X) ->
    Y = ".",
    fun() ->
            X1 = Y ++ ".",
            case X1 =:= X of
                true ->
                    X1;
                false ->
                    error({badmatch,X})
            end
    end(),
    ok.

The variable X1 is only needed for the matching. It will be
annotated with a compiler_generated annotation to ensure that no
compiler warnings will be generated if it would later turn out that it
would never be used.

Next constants are propagated and constant expressions are evaluated.
Since the fun is only used once, it will be eliminated and its body
inlined. Now we have:

foo(X) ->
    case ".." =:= X of
        true ->
            "..";
        false ->
            error({badmatch,X})
    end,
    ok.

The final optimization for this example is simplification of expressions
whose values are never used:

foo(X) ->
    case ".." =:= X of
        true ->
            ok;  %Warning: a term is constructed, but never used
        false ->
            error({badmatch,X})
    end,
    ok.

The ".." string will never be used, so it will be replaced with ok.
At the same time, a warning will be emitted.

The reason for the warning is that the ".." string does not have a
compiler_generated annotation to indicate that it was introduced by
the compiler. The X1 variable had a compiler_generated annotation,
but it was lost when X1 was replaced with "..".

To eliminate the unwanted warning, the compiler_generated annotation
must be propagated from the variable to the substituted value.

Thanks to Jose Maria Perez Ramos (@Kuroneer) for noticing this bug and
suggesting a way to fix it.

In OTP 24, there will be a warning for the following code:

    foo(X) ->
        Y = ".",
        fun() ->
                X = Y ++ "."  %Warning: a term is constructed, but never used
        end(),
        ok.

The warning is incorrect, because the built term will be matched to
the previous value of `X`. This commit eliminates the warning.

To explain the bug, I will illustrate how the example program is transformed
by the Core Erlang passes. My illustrative examples will use Erlang instead
of Core Erlang.

The first transformation makes the match `X = Y ++ "."` explicit:

    foo(X) ->
        Y = ".",
        fun() ->
                X1 = Y ++ ".",
                case X1 =:= X of
                    true ->
                        X1;
                    false ->
                        error({badmatch,X})
                end
        end(),
        ok.

The variable `X1` is only needed for the matching. It will be
annotated with a `compiler_generated` annotation to ensure that no
compiler warnings will be generated if it would later turn out that it
would never be used.

Next constants are propagated and constant expressions are evaluated.
Since the fun is only used once, it will be eliminated and its body
inlined. Now we have:

    foo(X) ->
        case ".." =:= X of
            true ->
                "..";
            false ->
                error({badmatch,X})
        end,
        ok.

The final optimization for this example is simplification of expressions
whose values are never used:

    foo(X) ->
        case ".." =:= X of
            true ->
                ok;  %Warning: a term is constructed, but never used
            false ->
                error({badmatch,X})
        end,
        ok.

The `".."` string will never be used, so it will be replaced with `ok`.
At the same time, a warning will be emitted.

The reason for the warning is that the `".."` string does not have a
`compiler_generated` annotation to indicate that it was introduced by
the compiler. The `X1` variable had a `compiler_generated` annotation,
but it was lost when `X1` was replaced with `".."`.

To eliminate the unwanted warning, the `compiler_generated` annotation
must be propagated from the variable to the substituted value.

Thanks to Jose Maria Perez Ramos (@Kuroneer) for noticing this bug and
suggesting a way to fix it.
@bjorng bjorng added team:VM Assigned to OTP team VM fix testing currently being tested, tag is used by OTP internal CI labels May 27, 2021
@bjorng bjorng self-assigned this May 27, 2021
@paulo-ferraz-oliveira
Copy link
Contributor

Hm... I wonder if this fix will have an impact on what @michaelklishin describes in erlang-lager/lager#547.

@bjorng
Copy link
Contributor Author

bjorng commented May 28, 2021

Hm... I wonder if this fix will have an impact on what @michaelklishin describes in erlang-lager/lager#547.

Yes, this bug could explain that there were still warnings even though compiler_generated was used.

@bjorng bjorng merged commit de53a08 into erlang:maint May 28, 2021
@bjorng bjorng deleted the bjorn/compiler/eliminate-spurious-warning/OTP-17446 branch May 28, 2021 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants