Repository navigation
Conversation
…iler
Hover, completion, signature help, and go to definition all go through
`v -vls-mode -line-info`, which the V launcher routes to the V1
compatibility compiler. When that compiler is missing, the launcher
refuses with a single line and exits:
`-vls-mode` requires the compatibility compiler, but no usable V
0.5.2 fallback was found and make is unavailable. Install make,
then run `make v1` in `C:\Users\me\v`.
That refusal is not an "unknown option" line, so neither
`compiler_rejects_line_info` nor `compiler_refused_and_stopped`
recognized it. `line_info_mode` stayed `.direct`, so VLS kept spawning
the compiler once per request for an answer that can never arrive, and
every one of those lookups resolved to empty with nothing said about why.
Recognize the refusal, retire the lookups as `.missing` so they are
answered from VLS's own index instead of paying a process launch each,
and tell the user what to install. A launcher that can build the
fallback itself announces "running `make v1` now" and then answers, so
that form is explicitly not a dead end.
The notice needs no "already warned" flag: the caller sets
`line_info_mode` to `.missing` first, and from then on `run_v_line_info`
returns from its early `.missing` check without reaching this point, so
it is sent at most once per session.
vlang/v#29369 (merged as 76d88b9) rewrites the tail of the launcher's refusal: the sentence after "make is unavailable" is now a platform-specific hint, and the comma became a period. On Windows the clause reads On Windows, install GNU make in MSYS2 (`make` or `mingw32-make`) and put its tools, including `sh`, on PATH. and elsewhere it is still "Install make.". `compiler_lacks_compatibility_compiler` keys on "requires the compatibility compiler", which both spellings contain, so detection is unaffected. This test pinned the old wording verbatim, so it now asserts both: green before the compiler change and after it, and the coupling is written down instead of being rediscovered the next time the wording shifts. Validated against V 0.5.2 76d88b9, the merged compiler: the Windows sample above is its output verbatim. `interop_test.v` passes; the module suite reports the same 3 failures with and without this change - `index_test.v`, `handlers_test.v` and `integration_test.v`, none of them in this file. `integration_test.v` fails on an empty completion list, which is the symptom of the missing V1 fallback this refusal describes, and is what vlang/v#29369 and vlang/vscode-vlang#543 are about.
…tics - Use compile_target as exec_dir and diagnostic_source_dir so the compiler checks the correct module directory - Add base_dir parameter to source_path_from_overlay so relative paths resolve against the compile target, not the project root - Escape backslashes in JSON rootPath for Windows paths - Fix u64 to int conversion in test file size calculations
When the 'v' on PATH is a forwarding wrapper (v.bat -> v.exe), find_v_dir returned the wrapper directory, so vlib module lookups (os, builtin, time, strings) resolved nowhere and indexed member completions came back empty. Unwrap .bat/.cmd shims to their quoted executable target in resolve_v_compiler_exe and cover it with tests.
…sing-compat-compiler
…h split_check_errors
…update source_path_from_overlay callers
Every edit now schedules two jobs: a fast index-only unknown-import check (150ms debounce) and the full compiler check (800ms). The slow answer replaces the fast one, so clean files see no extra traffic while a bad import is flagged in milliseconds. Adds opt-in VLS_PERF_LOG timing for requests and diagnostics jobs, plus the comparison/roadmap notes behind the change.
run_v_check fingerprints the program (compiler id plus sorted per-file content hashes, buffers winning over disk) before overlay or compiler work. Identical states reuse answers from memory or a per-program JSON file in the OS cache dir, across restarts. Reopening this repo unchanged answers in about 15ms from disk instead of 5s cold. Also hardens compiler spawn: a missing compiler returns an error result instead of crashing the server.
find_v_dir_from_exe walks up to a directory holding vlib. The fake compiler tree had none, so there was no V home to find and the test failed everywhere. Creating base/vlib mirrors a real installation.
run_v_line_info_once tries the warm shared server and its program copy first, falling back to the one-shot compiler path on any failure. Compat and missing modes keep their existing path untouched, and the pool never changes mode probing. Hover, signature help, and compiler-backed completions stop paying overlay plus spawn per request when a server is warm.
Extends the compiler-free fast check with bracket/string/comment/interpolation balance scanning that mirrors the index tokenizer. Only certain findings publish (stray closers, unclosed constructs at EOF with no continuation); mid-keystroke states stay silent and the slow check corrects the rest.
Stub compilers for modern-direct, old-flag, and dead-end generations assert the probed LineInfoMode, diagnostics results, and mode stability across repeated probes.
About 115 new focused tests for previously uncovered pure and harness-level helpers: occurrences, declarations, completions, positions, import parsing, fingerprint and disk-cache paths, JSON framing, inlay stamps. Coverage heuristic up from 38 percent to about 71 percent of functions mentioned in tests.
…path collisions Direct tests for the remaining pure helpers in rename, rename_conflicts, module_imports (now fully covered) and expression_types. make_unique_temp_path gains a per-process counter suffix: the Windows clock returned the same tick 19987 times in 20000 probes, so pid plus timestamp collided by construction.
Issues 474 and 473 both reproduce clean on this tree (compile-target rework and sibling-aware completion cover them), so these are guards with breaking controls: a nested two-file program checks clean and maps diagnostics to real paths, and an unopened sibling resolves through completion, definition, diagnostics, and didOpen.
run_v_check logs one VLS_PERF_LOG line per slow check with fingerprint, overlay prep, compiler run, and parse milliseconds. First measurement on this host: the compiler run is 97 percent of a 360ms single-file check.
reindex_uri capped disk files at 2MB but indexed open buffers of any size, allowing one huge buffer to force an unbounded index build. The gate now covers both paths; oversized entries are dropped and marked skipped exactly like oversized disk files.
The overlay directory persists per program instead of being wiped per check; buffers rewrite only when their content hash differs, and stale copies are pruned. Warm prep time measured 90ms down to 21ms on a 52-file overlay.
vls.inlayHints gains variableTypes and parameterNames options matching the two kinds the compiler emits. Defaults preserve current behavior; runtime toggles apply without restart.
Per-symbol trigram caches maintained incrementally inside index entries, ranked by overlap then match class, case and underscore insensitive with a bounded per-symbol cap.
Inserting thousands of entries one by one doubles the map backing store repeatedly; a late doubling can abort under a fragmented heap (GC_alloc_large on big trees). Reserve once from the walk's file count.
reindex_uri capped disk files at 2MB but indexed open buffers of any size, allowing one huge buffer to force an unbounded index build. The gate now covers both paths; oversized entries are dropped and marked skipped exactly like oversized disk files.
Inserting thousands of entries one by one doubles the map backing store repeatedly; a late doubling can abort under a fragmented heap (GC_alloc_large on big trees). Reserve once from the walk's file count.
Renaming to a name already live in the scope, or renaming a member of an interface its implementations must keep, is refused with a message naming the conflict, instead of silently producing code that will not compile.
The rule is a comment on the line the selection starts on, and the region searched began after that line so the rule's own text could not be matched. That also excluded the code *before* the comment on the same line, which is the natural way to write it: annotate the call you want rewritten and select it. `f(1) // f($a) ==>> g($a)` offered nothing at all, with nothing said about why. The region searched is now the selection, and a match that reaches into the rule's comment is dropped instead. The interval test is `start < end of the rule's line` and `end > start of its comment`, so a match ending exactly at the comment — the call before it — is kept. Both directions are pinned: a test that the call is offered, and the existing one that the comment itself is not rewritten, which now fails without the drop.
…iler
The fingerprint hashes a file's bytes, so rewriting the words of a
comment — the commonest edit that provably cannot change a diagnostic —
invalidates the cached answer and pays a full compiler check.
The fingerprint now hashes the file's code with its comments dropped:
diag_code_sketch keeps every newline a comment held, because those move
every line under them, and drops the rest of the comment. An edit to the
words changes nothing; an edit that adds a line still does, since every
line below moves and a diagnostic's line number moves with it.
A string's body is kept verbatim, so an interpolation inside one stays
code to the fingerprint and editing it still reaches the compiler. That
is why the sketch is not the existing masked_v_code, which blanks string
bodies: an edit inside `${...}` must not be free, because reusing an
answer there would leave a stale error on screen.
The sketch is cache-key material rather than an analysis input, so it
does not preserve offsets — which is what makes a comment's length
irrelevant. The first version did preserve them, and thereby made an
edit to a comment's length cheap but not its words, which is the case
that actually happens.
Two tests, both controlled by turning the dropping off: a comment edit
leaves the fingerprint alone and an interpolation edit does not.
An action carried an edit, which is computed by the server. Writing a test body is not that kind of work: the file is written by `v cover --write-tests`, and the server only points at the function. LSP has a field for exactly this, so add it rather than smuggle the command through an edit. `main.v` advertises the new command alongside the two it already had.
A test-generation action has to know which functions a run never entered, and that answer lives in a coverage directory. The tiers that decide what a check runs with are the natural place to ask, so a fourth key joins them: the editor's own settings, the project's `vls.json`, the environment, and nothing. The editor tier wins, which is what lets an extension that already runs coverage point VLS at the directory it recorded into instead of running the tests a second time.
A coverage recording is what tells VLS which functions a run never entered. With a coverage directory configured, a `.v` file offers one action per such function, plus one for the whole file when there is more than one. The action runs `v cover --write-tests`, which is the one tool that generates the body; the server names the file and reports what the tool wrote, and never composes a test itself. What it writes is a starting point and not a test: a call with zero values and a `// TODO:` line, because the tool cannot know what a function should return. `-no-skip-unused` belongs with the recording and is documented as such. Without it a function nothing calls is dropped from the build, so its coverage cannot be reported at all. No directory configured means no action offered, rather than an action that opens a message: a directory that does not exist gives the same answer, and the report it produces is read through a decodable struct so a newer report cannot break an older server.
hover_result flushed the diagnostics debounce on every request, starting a full compile concurrently with the compiler work hover itself runs. None of hover's reads depend on the flushed state (v3 asks with buffer content, the index reads as it stands, the one-shot writes the buffer to its own overlay), so the flush bought nothing for the answer and only contended with it. The debounced slow check still publishes on its own afterwards.
…Test The fixture builds its own Response with two commands but asserts three; it failed regardless of server behavior. a8fb888 added the command to the live initialize literal without updating this duplicate.
`stop()` writes `quit`, waits a second in 10ms steps, then kills the server, so it returns in about a second. The test allowed five, and the macOS job failed it twice: the assertion is a deadline, not a mechanism, and a runner compiling the other test files stretched that second of sleeping past it. The same tree passes this test on ubuntu, and the macOS job on this branch fails on it with nothing else changed, so this is the runner, not stop(). Thirty seconds is a ceiling on a freeze, which is what the test is for: a `stop()` that really waits for the server never returns, so the tighter number was only ever measuring how busy the machine was.
This was referenced Oct 19, 2026
Contributor
|
Findings P1: Pull diagnostics never receive async results. P1: Workspace diagnostics cannot clear stale errors. P1: Answer-budget follow-ups send duplicate JSON-RPC responses. P1: Configuration leaks between TCP clients. P1: Rename can proceed after losing an oversized unsaved-file marker. P1: The PR test suite has a new inlay-hints failure. |
Contributor
Author
|
Acknowledged all 6 P1 findings from @JalonSolov. I've started work on them; I'll push fixes in follow-up commits on this branch rather than amending. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Combines #546 and #547 on one branch that merges cleanly into master.
#546 (feat/tiered-diagnostics) is tiered diagnostics plus everything built
on it: a fast unknown-import check ahead of the compiler, a persistent
fingerprint cache, pooled line-info, refactors and assists, layered
configuration, and protocol additions. #547 (fix/index-memory-ci) fixes the
CI index abort: size gates on open buffers, a pre-sized symbol index, and
completeness for buffers that were never saved.
Most of #547's review findings were already carried over, so its remaining
delta is small: a
max_filesceiling on the project walk(
collect_v_filestakes the budget,project_walk_budget()reads it,index_walk_budgetlowers it in tests), reserve counting partial walks,and unsaved-buffer completeness in both
index_is_completechecks. Mergeconflicts stayed inside overlapping comments and one same-named test kept
in its newer budget-seam form; the duplicated
index_reserve_targetdefinition from the overlap is gone, and the background refresh walk site
follows the same budget convention.
RAM/CPU: the symbol map is pre-sized once before bulk and partial walks
instead of doubling repeatedly under a fragmented heap (the abort this
fixes); walks honor a file ceiling with a test seam instead of needing
twenty-thousand-file fixtures; open buffers and per-keystroke occurrence
maps share the 2 MiB disk gate so one huge buffer cannot force an
unbounded build; an unsaved buffer still counts as not-indexed, so a
destructive operation cannot proceed over references it never saw.
Verified on this host:
v -checkclean,v fmt -verifyclean, full suite20 passed / 1 skipped / 3 failed out of 24. The 3 failures (integration,
snapshot, handlers completion and rename cases) are the missing V1
compatibility compiler — this host has no
maketo build it — and failbyte-identically with and without this merge, confirmed by running the
same three files on the pre-merge base.
index_test.vis fully greenincluding the merged reserve, budget, and gate tests, and #547's new
rename-refusal test passes. The skipped file is platform-gated
(
v3_line_info_test.v,!windows). CI jobs were still queued behind theshared runner backlog at push time; the 3 local failures need the V1
compiler CI builds via
make v1.