Skip to content

checker,transformer: add always true false branch detect#25674

Merged
spytheman merged 16 commits into
vlang:masterfrom
kbkpbot:fix-checker-add-always-true-false-branch-check
Nov 9, 2025
Merged

checker,transformer: add always true false branch detect#25674
spytheman merged 16 commits into
vlang:masterfrom
kbkpbot:fix-checker-add-always-true-false-branch-check

Conversation

@kbkpbot

@kbkpbot kbkpbot commented Nov 5, 2025

Copy link
Copy Markdown
Contributor

feature request by #23404
This will warn on always true cond, such as a == a, and always false cond, such as 1==2.

Fix transformer support ast.CharLiteral expr, support general var such as struct.f != struct.f

Comment thread vlib/v/checker/if.v Outdated
@kbkpbot kbkpbot changed the title checker: add always true false branch detect checker,transformer: add always true false branch detect Nov 6, 2025
@kbkpbot kbkpbot marked this pull request as draft November 6, 2025 08:19
Comment thread vlib/v/checker/if.v Outdated
Comment thread vlib/v/checker/match.v Outdated
@kbkpbot

kbkpbot commented Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

CI fail is because in vtcc/src/i386-asm.v, there are too many stmts like:

        ASMInstr{
                sym:        u16(Tcc_token.tok_asm_shldw)
                opcode:     (u16((if (((4005) & 65280) == 3840) {
                        ((((4005) >> 8) & ~255) | ((4005) & 255))
                } else {
                        (4005)
                })))
                instr_type: u16(((8 | 4096) | ((0) << 13) | (if (((4005) & 65280) == 3840) { 256 } else { 0 })))
                nb_ops:     3
                op_type:    [u8(opt_cl), opt_regw, opt_ea | opt_regw]!
        },

and if (((4005) & 65280) == 3840) is always a constant expr and will eval to true/false.
This will cause the total notice number exceed the limit:

builder error: too many errors/warnings/notices

@kbkpbot kbkpbot marked this pull request as ready for review November 9, 2025 00:47
@spytheman

Copy link
Copy Markdown
Contributor

What is the performance impact of the additional checks?
Can you please run v run .github/workflows/compare_pr_to_master.v ?

@kbkpbot

kbkpbot commented Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

What is the performance impact of the additional checks? Can you please run v run .github/workflows/compare_pr_to_master.v ?

It need more optimization. :)

 >  1           base            114.4ms ± σ:    0.6ms,  113.6ms… 115.1ms `./vold -no-parallel -o ohw.exe examples/hello_world.v`
    2     +7.6%   1.08x slower  123.1ms ± σ:    0.5ms,  122.4ms… 123.6ms `./vnew -no-parallel -o nhw.exe examples/hello_world.v`
>>>>>> size("          nhw.exe") - size("          ohw.exe") =     240697 -     240697 =          0
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 13079.494 ms
 >  1           base            536.1ms ± σ:    1.5ms,  534.0ms… 537.2ms `./vold -check-syntax           cmd/v`
    2     +1.0%   1.01x slower  541.3ms ± σ:    2.4ms,  538.7ms… 544.5ms `./vnew -check-syntax           cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 13119.916 ms
 >  1           base            537.4ms ± σ:    1.7ms,  535.0ms… 539.0ms `./vold -check-syntax           cmd/v`
    2     +1.8%   1.02x slower  547.0ms ± σ:    1.5ms,  545.9ms… 549.1ms `./vnew -check-syntax           cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 13135.491 ms
    1     -0.1%   1.00x ~same~  542.6ms ± σ:    2.7ms,  538.8ms… 545.1ms `./vnew -check-syntax           cmd/v`
 >  2           base            542.9ms ± σ:    0.9ms,  541.6ms… 543.8ms `./vold -check-syntax           cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 22293.612 ms
 >  1           base            878.6ms ± σ:    2.3ms,  875.9ms… 881.4ms `./vold -check                  cmd/v`
    2     +9.2%   1.09x slower  959.3ms ± σ:   12.2ms,  942.2ms… 969.6ms `./vnew -check                  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 22410.240 ms
 >  1           base            885.1ms ± σ:    2.7ms,  881.6ms… 888.3ms `./vold -check                  cmd/v`
    2     +9.4%   1.09x slower  968.0ms ± σ:    2.6ms,  964.4ms… 970.4ms `./vnew -check                  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 22377.652 ms
 >  1           base            882.5ms ± σ:    1.8ms,  880.6ms… 885.0ms `./vold -check                  cmd/v`
    2     +9.7%   1.10x slower  967.8ms ± σ:    0.7ms,  966.9ms… 968.6ms `./vnew -check                  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 34632.271 ms
 >  1           base           1381.1ms ± σ:    1.8ms, 1378.8ms…1383.2ms `./vold -no-parallel -o ov.c    cmd/v`
    2     +7.6%   1.08x slower 1485.8ms ± σ:    2.7ms, 1482.2ms…1488.7ms `./vnew -no-parallel -o nv.c    cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 34627.267 ms
 >  1           base           1378.3ms ± σ:    3.3ms, 1373.7ms…1381.2ms `./vold -no-parallel -o ov.c    cmd/v`
    2     +7.5%   1.08x slower 1481.9ms ± σ:    5.1ms, 1474.6ms…1486.2ms `./vnew -no-parallel -o nv.c    cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 34602.570 ms
 >  1           base           1382.0ms ± σ:    2.6ms, 1378.7ms…1385.1ms `./vold -no-parallel -o ov.c    cmd/v`
    2     +7.5%   1.08x slower 1485.9ms ± σ:    3.2ms, 1481.4ms…1489.0ms `./vnew -no-parallel -o nv.c    cmd/v`
>>>>>> size("             nv.c") - size("             ov.c") =    6778937 -    6779002 =        -65
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 40363.023 ms
 >  1           base           1616.9ms ± σ:    7.6ms, 1606.9ms…1625.4ms `./vold -no-parallel -o ov.exe  cmd/v`
    2     +6.8%   1.07x slower 1726.1ms ± σ:    2.7ms, 1722.4ms…1728.9ms `./vnew -no-parallel -o nv.exe  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 40266.581 ms
 >  1           base           1617.5ms ± σ:    7.7ms, 1606.7ms…1623.7ms `./vold -no-parallel -o ov.exe  cmd/v`
    2     +5.9%   1.06x slower 1713.2ms ± σ:   10.6ms, 1698.3ms…1722.6ms `./vnew -no-parallel -o nv.exe  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 40190.929 ms
 >  1           base           1616.3ms ± σ:    1.6ms, 1614.0ms…1617.8ms `./vold -no-parallel -o ov.exe  cmd/v`
    2     +6.3%   1.06x slower 1718.7ms ± σ:    1.6ms, 1716.5ms…1720.1ms `./vnew -no-parallel -o nv.exe  cmd/v`

@kbkpbot kbkpbot marked this pull request as draft November 9, 2025 09:19
@kbkpbot

kbkpbot commented Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

Is this acceptable ?

Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 2784.369 ms
 >  1           base            113.6ms ± σ:    0.3ms,  113.2ms… 113.9ms `./vold -no-parallel -o ohw.exe examples/hello_world.v`
    2     +2.4%   1.02x slower  116.3ms ± σ:    0.1ms,  116.2ms… 116.4ms `./vnew -no-parallel -o nhw.exe examples/hello_world.v`
>>>>>> size("          nhw.exe") - size("          ohw.exe") =     240697 -     240697 =          0
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 13052.584 ms
 >  1           base            538.7ms ± σ:    2.1ms,  535.7ms… 540.6ms `./vold -check-syntax           cmd/v`
    2     +0.3%   1.00x slower  540.1ms ± σ:    0.8ms,  539.0ms… 540.8ms `./vnew -check-syntax           cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 13080.751 ms
 >  1           base            540.9ms ± σ:    5.3ms,  533.4ms… 544.9ms `./vold -check-syntax           cmd/v`
    2     +0.1%   1.00x ~same~  541.5ms ± σ:    0.4ms,  540.9ms… 541.9ms `./vnew -check-syntax           cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 13073.723 ms
 >  1           base            537.6ms ± σ:    5.5ms,  531.2ms… 544.5ms `./vold -check-syntax           cmd/v`
    2     +0.5%   1.01x slower  540.6ms ± σ:    2.1ms,  537.7ms… 542.1ms `./vnew -check-syntax           cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 21559.154 ms
 >  1           base            879.6ms ± σ:    3.4ms,  875.6ms… 883.9ms `./vold -check                  cmd/v`
    2     +2.8%   1.03x slower  904.2ms ± σ:    0.4ms,  903.8ms… 904.7ms `./vnew -check                  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 21655.860 ms
 >  1           base            886.1ms ± σ:    1.7ms,  884.7ms… 888.5ms `./vold -check                  cmd/v`
    2     +2.4%   1.02x slower  907.6ms ± σ:    0.2ms,  907.4ms… 907.9ms `./vnew -check                  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 21585.384 ms
 >  1           base            880.4ms ± σ:    2.1ms,  878.4ms… 883.4ms `./vold -check                  cmd/v`
    2     +1.8%   1.02x slower  896.7ms ± σ:    7.3ms,  886.4ms… 902.1ms `./vnew -check                  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 33666.340 ms
 >  1           base           1379.6ms ± σ:    2.2ms, 1376.7ms…1381.9ms `./vold -no-parallel -o ov.c    cmd/v`
    2     +1.9%   1.02x slower 1406.5ms ± σ:    2.5ms, 1402.9ms…1408.8ms `./vnew -no-parallel -o nv.c    cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 33742.275 ms
 >  1           base           1378.8ms ± σ:    0.9ms, 1378.0ms…1380.0ms `./vold -no-parallel -o ov.c    cmd/v`
    2     +2.5%   1.03x slower 1413.4ms ± σ:    5.6ms, 1406.4ms…1420.1ms `./vnew -no-parallel -o nv.c    cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 33671.153 ms
 >  1           base           1381.3ms ± σ:    1.7ms, 1379.3ms…1383.5ms `./vold -no-parallel -o ov.c    cmd/v`
    2     +1.5%   1.02x slower 1402.1ms ± σ:    3.7ms, 1396.9ms…1405.7ms `./vnew -no-parallel -o nv.c    cmd/v`
>>>>>> size("             nv.c") - size("             ov.c") =    6778808 -    6778873 =        -65
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 1/3, took: 39562.733 ms
 >  1           base           1619.1ms ± σ:    2.3ms, 1617.0ms…1622.2ms `./vold -no-parallel -o ov.exe  cmd/v`
    2     +1.8%   1.02x slower 1648.9ms ± σ:    5.8ms, 1640.7ms…1654.0ms `./vnew -no-parallel -o nv.exe  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 2/3, took: 39361.583 ms
 >  1           base           1620.9ms ± σ:    2.8ms, 1618.7ms…1624.8ms `./vold -no-parallel -o ov.exe  cmd/v`
    2     +1.3%   1.01x slower 1641.8ms ± σ:    5.9ms, 1633.4ms…1646.1ms `./vnew -no-parallel -o nv.exe  cmd/v`
Summary after 1 series x 10 runs (%s are relative to first command, or `base`), discard maxs:  7, repeat: 3/3, took: 39381.921 ms
 >  1           base           1617.8ms ± σ:    1.3ms, 1616.6ms…1619.6ms `./vold -no-parallel -o ov.exe  cmd/v`
    2     +1.7%   1.02x slower 1645.5ms ± σ:    2.2ms, 1642.8ms…1648.2ms `./vnew -no-parallel -o nv.exe  cmd/v`

@kbkpbot kbkpbot marked this pull request as ready for review November 9, 2025 14:33
@spytheman

Copy link
Copy Markdown
Contributor

Is this acceptable ?

Yes, 1-2% hit is a lot more palatable.
Thank you.

@spytheman spytheman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work.
Thank you @kbkpbot 🙇🏻‍♂️ .

@spytheman spytheman merged commit 9760a9c into vlang:master Nov 9, 2025
84 checks passed
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.

4 participants