[vim/vim] vim9: skip elseif condition when in dead branch (PR #20021)

2 views
Skip to first unread message

Furkan Sahin

unread,
Apr 19, 2026, 3:23:51 PM (yesterday) Apr 19
to vim/vim, Subscribed

Problem: When an if condition is constant true, the else block is skipped during compilation. However, any elseif condition within that skipped block was still compiled. This caused errors when the condition referenced variables only declared in the skipped block or when it checked for missing features (like has('clipboard')).

Solution: In compile_elseif(), check if ctx_skip is already set to SKIP_YES. If so, skip compiling the entire condition expression using skip_expr_cctx() and avoid processing it further.

fixes: #19160


You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/20021

Commit Summary

  • f9de67c vim9: skip elseif condition when in dead branch

File Changes

(2 files)

Patch Links:


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021@github.com>

Christian Brabandt

unread,
Apr 19, 2026, 3:56:54 PM (yesterday) Apr 19
to vim/vim, Subscribed

@chrisbra commented on this pull request.


In src/testdir/test_vim9_script.vim:

> @@ -5933,4 +5933,20 @@ def Test_g_variable_shadow_multi_context()
   unlet g:F
 enddef
 
+" Test that dead elseif conditions are skipped without errors
+def Test_skip_elseif_dead_branch()
+  if true
+    echo 'ok'
+  else
+    var x = 0
+    if x > 0
+      echo 'pos'
+   # This condition should never be compiled
+    elseif x < 0
+      echo 'neg'
+    endif
+  endif
+enddef
+defcompile

Please follow the existing pattern in this file here:
see the first function:

def Test_vim9script_feature()
  # example from the help, here the feature is always present
  var lines =<< trim END
      " old style comment
      if !has('vim9script')
        " legacy commands would go here
        finish
      endif
      vim9script
      # Vim9 script commands go here
      g:didit = true
  END
  v9.CheckScriptSuccess(lines)
  assert_equal(true, g:didit)
  unlet g:didit
enddef

That is, use the here document to write your test scripts and then use the v9.CheckScriptSuccess() functions to check the result on sourcing.

Please also add a test for the opposite issue, to make sure the else if is executed after the false expression.

def Test_skip_elseif_after_dead_then()
  var lines =<< trim END
    vim9script
    g:reached = 0
    if false
      echo 'never'
    elseif true
      g:reached = 1
    endif
  END
  v9.CheckScriptSuccess(lines)
  assert_equal(1, g:reached)
  unlet g:reached


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/review/4136471521@github.com>

Furkan Sahin

unread,
Apr 19, 2026, 4:40:36 PM (yesterday) Apr 19
to vim/vim, Push

@sahinf pushed 1 commit.

  • de213fd vim9: skip elseif condition when in dead branch


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/before/f9de67c4dd6559572fb7d42a814d2d044cd433c3/after/de213fd83821f1370373b7b1572e32f3d1246a77@github.com>

Furkan Sahin

unread,
Apr 19, 2026, 4:40:39 PM (yesterday) Apr 19
to vim/vim, Subscribed

@sahinf commented on this pull request.


In src/testdir/test_vim9_script.vim:

> @@ -5933,4 +5933,20 @@ def Test_g_variable_shadow_multi_context()
   unlet g:F
 enddef
 
+" Test that dead elseif conditions are skipped without errors
+def Test_skip_elseif_dead_branch()
+  if true
+    echo 'ok'
+  else
+    var x = 0
+    if x > 0
+      echo 'pos'
+   # This condition should never be compiled
+    elseif x < 0
+      echo 'neg'
+    endif
+  endif
+enddef
+defcompile

Oops done! I verified that the first test fails without the patch; though the 2nd test passes without it. Also we have to be inside a def block in the heredoc to hit the bug.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/review/4136516208@github.com>

h_east

unread,
Apr 19, 2026, 5:27:09 PM (yesterday) Apr 19
to vim/vim, Subscribed
h-east left a comment (vim/vim#20021)

Summary of findings

Request changes. The fix introduces a regression: if false / elseif true
no longer enters the elseif block. The condition guard
(cctx->ctx_skip == SKIP_YES) is too broad; it also triggers for the normal
case where the preceding if branch was statically false, which is exactly
when the elseif condition must be compiled and evaluated.

A one-line change to check the saved outer-scope skip state
(scope->se_skip_save == SKIP_YES) fixes both issue #19160 and avoids the
regression.

Reproduction of the reported symptom

The original reproduction from #19160 requires a Vim built with -clipboard.
I reproduced the essence of the bug using a constant-true outer if and a
nested if/elseif inside the dead else branch. Without the PR the compiler
still tries to compile the elseif condition inside the dead branch.
With the PR applied (master patch only) the original case no longer errors,
confirming the fix resolves the reported issue.

Regression found

Test case:

vim9script
var result = ''
def Bug()
  if false
    result = 'A'
  elseif true
    result = 'B (expected)'
  else
    result = 'C (bug)'
  endif
enddef
Bug()
writefile([result], '/tmp/bug.out')
  • On master: writes B (expected) (correct).
  • On this PR (dead-branch): writes C (bug) — the elseif branch is never
    entered; execution falls through to else.

Why this happens

In compile_elseif() (src/vim9cmds.c), ctx_skip == SKIP_YES arises in two
distinct situations:

  1. The preceding if condition was a compile-time constant false. In this
    case the preceding block body was skipped (correct), but the elseif
    condition must still be compiled because it may turn out to be true
    at compile time, or it may need to generate runtime checks. The existing
    code at lines 674-687 explicitly resets ctx_skip to SKIP_UNKNOWN and
    re-compiles the expression for exactly this reason.
  2. The entire enclosing if/else scope is dead (e.g. we are inside the
    else branch of a statically-true outer if). In this case the
    elseif and its condition are also dead, and compiling the condition is
    undesirable.

The PR conflates situations 1 and 2 by returning early whenever
ctx_skip == SKIP_YES. This breaks situation 1: the elseif condition is
skipped and no JUMP_IF_FALSE / end-label jumps are emitted, so the elseif
body effectively disappears from the generated code.

Existing tests don't catch this

Test_if_elseif_else only uses dynamic conditions
(if what == 1 / elseif what == 2 / ...), and Test_if_const_expr doesn't
exercise the if false / elseif <...> pattern. That's why the PR's
make test_vim9_script run appears green despite the regression.

Suggested fix

The compiler already records the outer ctx_skip on the if-scope via
scope->se_skip_save (see compile_if() in src/vim9cmds.c and existing
checks at lines 709, 793, 858 that already use this field to detect a dead
outer scope). Using the same signal in compile_elseif() cleanly
distinguishes situation 2 from situation 1:

-    if (cctx->ctx_skip == SKIP_YES)
+    if (scope->se_skip_save == SKIP_YES)
     {
-        // Already skipped a block so skip this condition without compiling
+        // The enclosing outer block is dead, so skip this elseif condition
+        // without compiling it.
         skip_expr_cctx(&p, cctx);
         return p;
     }

I verified this locally:

  • Issue #19160 original reproduction: still works (no compile error).
  • if false / elseif true / else regression: returns B (expected).
  • make test_vim9_script: 162 tests pass, including the PR's new tests
    Test_skip_elseif_dead_branch and Test_skip_elseif_with_has_in_dead_branch.

Additional suggestions

1. Add a regression test for the situation above

To prevent future breakage, please add a test that covers the non-dead
if false / elseif true / else pattern as well:

def Test_if_false_elseif_true_still_takes_elseif()
  
var lines =<< trim END
    vim9script
    var result = ''
    def F()
      if false
        result = 'A'
      elseif true
        result = 'B'
      else
        result = 'C'
      endif
    enddef
    F()
    assert_equal('B', result)
  END
  v9.CheckScriptSuccess(lines)
enddef

2. Test comment wording (minor)

The test comment says "inside a :def function" but the test body actually
puts everything inside a script with a nested :def. Consider adjusting
the comment (or dropping it) for accuracy. Not blocking.

Verdict

Request changes. The current patch fixes #19160 but breaks a common
existing pattern (if false / elseif true). Switching the guard to
scope->se_skip_save == SKIP_YES resolves both concerns with a minimal
change.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/c4276854845@github.com>

Furkan Sahin

unread,
Apr 19, 2026, 5:36:59 PM (yesterday) Apr 19
to vim/vim, Push

@sahinf pushed 1 commit.

  • 7394c2f vim9: skip elseif condition when in dead branch


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/before/de213fd83821f1370373b7b1572e32f3d1246a77/after/7394c2f8bfed8c32bf3bb05d046f4fc1153e9c80@github.com>

Christian Brabandt

unread,
12:23 PM (6 hours ago) 12:23 PM
to vim/vim, Subscribed
chrisbra left a comment (vim/vim#20021)

thanks both


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/c4282512754@github.com>

Christian Brabandt

unread,
12:31 PM (6 hours ago) 12:31 PM
to vim/vim, Subscribed

Closed #20021 via f74a416.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/20021/issue_event/24685987973@github.com>

Reply all
Reply to author
Forward
0 new messages