fix: three jsval-representation gaps in checked-dynamic lowering - #203
Open
techfreaque wants to merge 92 commits into
Open
fix: three jsval-representation gaps in checked-dynamic lowering#203techfreaque wants to merge 92 commits into
techfreaque wants to merge 92 commits into
Conversation
Update README to reflect changes in build output path.
The runtime assumed mingw-w64 on Windows, which provides POSIX headers and functions (ssize_t, clock_gettime, nanosleep, dirent.h, unistd.h) that MSVC's CRT does not ship. Users opening VS2022 Developer Command Prompt get MSVC's bundled clang (i686-pc-windows-msvc) instead of mingw-w64, and every compilation fails with missing type/function errors. Add _MSC_VER-guarded shims in scr_win.c: - clock_gettime() over QueryPerformanceCounter (monotonic) and GetSystemTimeAsFileTime (realtime) - nanosleep() over Sleep() - opendir/readdir/closedir over FindFirstFileW/FindNextFileW - CLOCK_REALTIME, CLOCK_MONOTONIC, struct timespec declarations Guard POSIX header includes in scr_lib.c, scr_path.c, scr_url.c with _MSC_VER checks, providing CRT equivalents (_getcwd, _access, _isatty) where needed. Fixes vercel-labs#25
Run on push/PR to fix/msvc-posix-shims only. Tests the exact scenario from vercel-labs#25: compiling the runtime with MSVC's bundled clang (no mingw, no zigcc) on windows-latest, including the Map + sort pattern that bare.ts uses. Also runs a Linux corpus smoke test to verify no regressions.
- Remove #include <windows.h> from scr_runtime.h's _MSC_VER block: it pulled winsock.h (via windows.h) into every TU, conflicting with winsock2.h included later in scr_lib.c - Guard struct timespec with #ifndef _TIMESPEC_DEFINED: modern UCRT already defines it, so redefinition caused C1104 errors on CI
The _TIMESPEC_DEFINED guard didn't work because UCRT's time.h defines struct timespec (line 45) but doesn't set _TIMESPEC_DEFINED in the clang/MSVC mode we're compiling in. Remove the definition entirely — UCRT 10.0.26100.0 provides it, and scr_win.c's clock_gettime/nanosleep use it without redefining.
MSVC CRT lacks several POSIX constants/types used throughout the runtime: - PATH_MAX (use _MAX_PATH from stdlib.h) - mode_t (typedef unsigned int) - F_OK (value 0) - S_ISDIR/S_ISREG macros (use _S_IFMT/_S_IFDIR/_S_IFREG from sys/stat.h) - S_ISLNK/S_ISFIFO/S_ISSOCK/S_ISBLK/S_ISCHR (stub as 0 on Windows) These are guarded by _MSC_VER so mingw-w64 and Linux are unaffected.
WideCharToMultiByte silently drops the null terminator when the UTF-8 output fills all 260 bytes of d_name. Force-terminate after the conversion to prevent out-of-bounds reads by callers.
- Mark node:crypto, node:zlib, and node:fs(+fs/promises) as partial: their island shims cover only a slice of Node's surface and the rest throw at the call. - Report partial shims as 'partial' in the --dynamic builtins table, with a note, so they are not indistinguishable from fully implemented shims. - Add a coverage snapshot for the crypto-shims fixture; full shims stay 'shimmed' (the esbuild-require snapshot pins that side).
refine() derived clearNaN from the negated operator, treating the failed edge of a < b as a >= b having held — the two differ exactly when a side is NaN, so guard-clause spellings let NaN be "proven" whole and cross a declared i64/u64 slot as an unchecked (int64_t) / fptosi conversion. Judge NaN exclusion by the relation that actually held on the edge; the numeric interval refinement is unchanged. Co-authored-by: Cursor <[email protected]>
Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01EYLKF6JBn2Fozts9CGr9W6
… kinds, C-emission mapping Adds declare module midi/node:midi, midiInput/midiOutput IR handle kinds, moduleUsesMidi predicate, type mapping, module registry entries, and the C-representation/retain/release mapping in the emission layer. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01EYLKF6JBn2Fozts9CGr9W6
Refcounted midi Input/Output handles over the event-loop poller seam, off-thread callback bridging via self-pipe, number[] message delivery with deltaTime, virtual-port loopback on POSIX, header decls and scr_async.c loop hook. Falls back to a stub backend where no MIDI stack is present. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01EYLKF6JBn2Fozts9CGr9W6
# Conflicts: # packages/runtime/src/scr_runtime.h # packages/runtime/src/scr_url.c
…h repo convention (vercel-labs#50)
… the pinned OPAQUE-global destructuring fence test and left the mechanism dead); keep only the well-scoped globalHints crypto message improvement
…e PR's own now-merged branch name, would never fire again)
…fixture (merge conflict fallout)
Found while pushing a large real-world program (next-vibe) through
--dynamic. All three are gaps where a value already in the `jsval`
(checked-dynamic island handle) representation hits a code path that
only recognized `dyn` or a handful of other kinds, not `jsval`.
1. **Union property reads on a jsval-lowered receiver crash the
compiler.** `lowerUnionProperty` (lower-exprs.ts) handles a
checker-union receiver whose VALUE lowered to `dyn`/`record`/`object`,
but not `jsval` (an optional-chained receiver whose narrowed arms all
trace back to one dynamic/npm value) — that case fell through to
`throw new Error("lowerer bug: union-typed receiver lowered to a
non-union")`, an unrecoverable process crash rather than a
diagnostic. Fixed by adding the missing `jsval` branch (the same
generic island property read `isIslandExpr`'s own jsval handling
already uses elsewhere in this file). Also improved the crash
message itself to include the offending kind, file, and line — it
previously gave no way to find the triggering code at all.
2. **A checked cast to a jsval-shaped target type incorrectly requires
JSON-boundary validation.** In the `as T` cast lowering, a jsval
receiver cast to a target type that ITSELF maps to `jsval` (an
npm/ambient-declared type with no static shape — e.g. many Drizzle
ORM query-builder return types) fell through the
`targetTs.flags & TypeFlags.Any` fast path (that check is for the
literal `any` keyword, not an alias that merely happens to map to
the same representation) into `boundarySafe`'s JSON-representable-
type validation, which a jsval target can never satisfy — producing
`error SC1090: a checked cast of 'any' to 'any' is not supported
yet` (formatIrType prints `jsval` as `"any"`, hence the confusing
doubled wording). Fixed by returning the receiver unchanged when the
target itself is jsval-shaped, matching the existing `Any`-keyword
fast path's semantics.
3. **A computed object-literal key that folds to a compile-time
constant string was rejected outright.** The 'any'-typed
object-literal lowering only accepted `Identifier`/`StringLiteral`
keys, rejecting the extremely common `{ [Methods.POST]: {...} }` /
`{ [SomeEnum.Member]: value }` shape (a computed key referencing an
enum member or other compile-time-constant string) even though this
file already has `literalComputedKey`/`foldedStringKeyOf` utilities
for exactly this fold, used by many other call sites in the same
file. Fixed by using the same fold here; `pushProp`'s signature
changed from `ts.Identifier | ts.StringLiteral` to a plain
`{ text: string; node: ts.Node }` pair so a folded computed key (no
single text-bearing AST node) can flow through it too.
## Verification
Compiled the full next-vibe program (~600 TypeScript files) through
`scriptc build --dynamic` before and after:
- Fix 1 eliminated an unconditional process crash (`lowerer bug:
union-typed receiver lowered to a non-union`) that previously
prevented the compiler from producing ANY diagnostic output at all
for this program.
- Fix 2 eliminated 256 instances of the doubled "any to any" error.
- Fix 3 eliminated 62 instances of the object-literal property-form
error (142 computed-key sites now resolve; the remaining error-count
delta reflects some of those unlocking further, unrelated downstream
diagnostics rather than a 1:1 elimination).
`packages/compiler` builds clean (`tsc -p tsconfig.json`, 0 errors).
Contributor
|
@techfreaque is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
13 participants
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.
Problem
Found while pushing a large real-world program (~600 TypeScript files) through
scriptc build --dynamic. Three separate gaps, all sharing the same shape: a value already in thejsval(checked-dynamic island handle) representation hits a code path that only recognizeddynor a handful of other kinds, notjsval.1. Union property reads on a jsval-lowered receiver crash the compiler outright.
lowerUnionPropertyhandles a checker-union receiver whose value lowered todyn/record/object, but notjsval— that case fell through tothrow new Error("lowerer bug: union-typed receiver lowered to a non-union"), an unrecoverable process crash (not a diagnostic) that stopped the whole compile from producing any output at all.2. A checked cast to a jsval-shaped target type incorrectly requires JSON-boundary validation.
x as TwhereTmaps tojsval(an npm/ambient-declared type with no static shape — e.g. many ORM query-builder return types) fell through thetargetTs.flags & TypeFlags.Anyfast path (that's for the literalanykeyword, not an alias that happens to map to the same representation) into JSON-boundary validation, which a jsval target can never satisfy — producing the confusingerror SC1090: a checked cast of 'any' to 'any' is not supported yet(formatIrTypeprintsjsvalas"any", hence the doubled wording).3. A computed object-literal key that folds to a compile-time constant string was rejected outright. The 'any'-typed object-literal lowering only accepted
Identifier/StringLiteralkeys, rejecting the common{ [Methods.POST]: {...} }/{ [SomeEnum.Member]: value }shape even though the file already hasliteralComputedKey/foldedStringKeyOfutilities for exactly this fold, used by many other call sites in the same file.Fix
jsvalbranch tolowerUnionProperty— the same generic island property read (jsOp/getProp)isIslandExpr's own jsval handling already uses elsewhere in the file. Also improved the crash message to include the offending kind/file/line, since it previously gave no way to find the triggering code.if (target.kind === "jsval") return inner;check, matching the existingAny-keyword fast path's semantics.pushProp's signature changed fromts.Identifier | ts.StringLiteralto{ text: string; node: ts.Node }so a folded computed key (which has no single text-bearing AST node of its own) can flow through it; a smallfoldedKeyTextOfhelper resolves identifier/string-literal/computed keys uniformly using the existing fold utilities.Verification
Compiled the full next-vibe program through
scriptc build --dynamicbefore and after each fix:packages/compilerbuilds clean (tsc -p tsconfig.json, 0 errors).