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.
A workspace pull reports every document a check has answered for, from the same cache the per-file pull reads, so no compiler runs and no second dedup or cap exists. An id the client already holds is answered unchanged, and the answer is bounded: at five hundred documents the report says how many were left out instead of silently dropping them.
… as incomplete index_reserve_target clamps to index_max_files instead of dropping the reservation when the projection crosses the cap; occurrences_for applies the same oversized-source gate as reindex_uri; index_is_complete_for_scope and index_is_complete treat a skipped open buffer with no file on disk as not indexed, so rename refuses instead of silently omitting references.
@medvednikov's review of the index-memory change asked for three things, and this branch predates those fixes. Ported across, adapted to this tree: index_reserve_target clamps to index_max_files instead of dropping the reservation when the projection crosses the cap; occurrences_for applies the same oversized-source gate reindex_uri uses; index_is_complete_for_scope and index_is_complete treat a skipped URI still in open_files as not indexed, so an unsaved oversized buffer stops a rename. Verified: v -check, v fmt -verify, and the full index_test.v green on Windows.
A cursor inside a nested call, on the inner paren, three levels deep, on a method chain and past every closing paren is what source_call_target answers, so the signature-help fallback cannot regress to reporting the outer function.
Adds `ssr.v`: a rule written as a comment on the line the selection
starts on, `pattern ==>> template`, with `$name` placeholders carrying
the text they matched. The action is offered as `refactor.rewrite` when
the selection sits on such a line.
Two invariants hold it together:
* Scope-negative. A rule is refused as a whole, and the reason is
logged, when any match sits in a `$if` branch, spans a `${...}`
interpolation, or starts where V writes a type — a struct field's
type, a generic argument, an annotation, or a return type.
* Visible. Every match is inside the selection, and the action's
preview is the edit set itself.
Three bugs found while making the tests green, each pinned by a test
that fails without the fix:
* The match's start was the offset the scan arrived from, so the edit
range began in the whitespace before the match and would have
replaced the preceding newline.
* The refusal read the whole document with spans relative to the
selected region, so a `$if` branch below the selection's start was
rewritten instead of refused.
* A trailing separator skip ate the pattern's own next token, so
`f($a, $b)` did not match `f(1, 2)`.
The render decides parentheses from the binding's own operators rather
than from a re-parse, because vls has no parser to re-read it with, and
a comma in the template is followed by a space as V code is written.
Deliberately absent, as each cannot be made safe on the index alone:
typed wildcards, rules that span files, and declaration-shape rewrites.
The full suite is green apart from nine failures that reproduce on a
clean checkout, where the V1 compatibility compiler is unavailable.
The check for a match inside an interpolation only looked for a `${`
inside the match's own span, so the common case — a pattern that
matched an interpolation's expression — was not refused at all. The
two bytes before the match are the opening `${` there, and the check
never looked there. `'a${x + f(1)}c'` matched with no refusal, and the
replacement would have been written inside the string.
`ssr_in_interpolation` scans the offset's own line instead: the quote
that opened the string and the brace depth inside it decide where the
interpolation runs, so a match anywhere inside one is caught, and a
plain `if x { f(1) }` still passes. A span that merely holds a `${` is
refused separately, because rewriting across a string breaks it.
The second branch of the old check claimed to stop a closing brace
being produced inside an interpolation, but its condition tested the
two bytes before the match for `}{`, which is not that. It is gone.
Also drops `SsrRefusal`, which nothing used.
Four defects found by running a shape matrix over the constructs a
structural replace touches, rather than by reasoning about them. Each
one is pinned by a test that fails without it.
A match inside `$else` was allowed. The comptime scan counted a `}`
that starts a line as a branch closing, but `} $else {` closes one
branch and opens the next on the same line, so the opener was never
settled and a match under it read as below the pair. The compiler does
not build the branch the flags do not choose, so a wrong rewrite there
is invisible to the slow check — the exact failure this layer exists to
prevent, in the one construct it did not name.
An attribute now refuses as an attribute. It was refused before, but
only because the prefix ended in `[`, which the type-position rule
answered — so the log told the user a match in `@[export]` was in a
type position, which is false and describes a different refusal.
An annotated field type was allowed. `item x T @[json: json]` ends with
an attribute rather than a bare name, so the field rule's last check
matched nothing and the type passed. The type is now the name the match
starts on rather than the last name on the line.
A match inside an array literal was refused as a type position. `[`
after `:=` opens a literal, not a generic argument. `[` after a name is
indistinguishable from an index by text alone, so both are refused;
`[` after an operator is now allowed.
The return-type rule no longer requires the line to open with `fn`,
which also covers an interface's `foo() T`.
Also fixes a comment on `ssr_find_matches` that described a
`code_only` parameter the function has never had.
…d-to-end test `ssr_skip_trivia` had an infinite loop. Its comment branch tested the loop bound on `pos`, the offset the function was called with, but incremented `at`; `pos` never moves, so the condition was always true and the loop ran until the counter wrapped. Every V file carries `//` comments, and the rule a structural replace reads is written as one, so the feature hung on its own usage. Found by the end-to-end test, which hung for fifteen minutes before it was killed. The rewrite also searched from the selection's start, whose region holds the rule's own comment, and the rule's text matches its pattern. A selection whose only match is that comment was offered a rewrite of it, which destroys the rule the user typed. The region now starts after the rule's line. Three tests pin these. The comment one hangs rather than fails when the loop returns, which is the only way an infinite loop can be pinned; a regression shows up as a run that does not finish, and CI's job timeout turns that red. The end-to-end test goes through `handle_code_action`, so it also pins the registration, the `refactor.rewrite` kind filter, and the LSP positions of the edits, none of which were covered before.
… an interpolation is not
Four cases were reasoned about and never measured.
`g($a + 1)` on `f(1 + 2)` renders `g((1 + 2)+1)`: the binding is
wrapped, which is the point, and the spacing is tight because the
tokenizer drops the rule's own spaces. `v fmt` unwinds the spacing, so
the tests assert the wrap rather than the whitespace, and say so.
`\${` is an escaped dollar, and a backtick string is raw, so `${` inside
either is text rather than an interpolation. A rewrite there is an
ordinary one and must not be refused: refusing it would make a rewrite
inside an escaped string impossible. Both are pinned with tests that
fail when the scanner starts counting those forms as interpolations.
The `$if` refusal now names `$else` too. It already refused a match in
one — `} $else {` closes a branch and opens the next on the same line —
but the message said only `$if`, which is the same imprecision the
attribute refusal was corrected for.
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.
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.