Port strada debug namespace, enable many debug assertions

[weswigham]

Almost every commented-out assertion in the codebase enables cleanly, except for three - two don't seem to apply anymore due to code structure changes and I've deleted them here, the third I've added a comment to and will enable as a follow-up alongside the fix to make it not immediately be triggered, since that'll have some baseline diffs accompanying it.

The conditional debug namespace members are all empty (or pass-thru where appropriate) functions when compiled with the release tag. That should make the compiler elide them and their arguments as dead code in that mode, which should make conditional assertions free in terms of shipping-release-performance. Unconditional members (fail, failBadSyntaxKind, assertNever) should be in-place both with and without the release tag.

Additionally, hereby now applies the release tag by default to build tasks. Tests aughta still get compiled without it, so should have full assertions. And if anyone builds from source or via the normal go build ./... command, they should also get assertions enabled by default. (The noembed tag we were already using already works like this, so this isn't exactly a new split).

There are a bunch of panics that should probably get reverted back into assert and fail calls at some point so they have a unified debug breakpoint inside Fail, but that can be a future task.

[Copilot]

Pull Request Overview

This PR ports the TypeScript Debug namespace to Go, enabling many debug assertions throughout the codebase that were previously commented out. The implementation provides conditional debug functions that are no-ops when compiled with the release tag, allowing assertions to be present in development but elided from production builds for performance.

Key changes include:

• Implementation of the internal/debug package with conditional compilation for debug vs release builds
• Enabling numerous previously commented-out debug assertions across the codebase
• Adding the release build tag to default hereby build tasks to optimize shipping releases

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/debug/shared.go	Unconditional debug functions (Fail, FailBadSyntaxKind, AssertNever) available in both builds
internal/debug/debug.go	Full debug assertion implementations active only in non-release builds
internal/debug/release.go	No-op implementations of debug functions for release builds
internal/ast/ast.go	Added helper methods KindString() and KindValue() to support debug assertions
Herebyfile.mjs	Added "release" tag to default build configurations
Multiple Go files	Enabled previously commented-out debug assertions and added debug package imports

Comments suppressed due to low confidence (1)

[Copilot]

This else block was removed but now the conditional structure may be incomplete. The removed assertion debug.AssertNever(node) suggests this else case should never be reached. Consider adding appropriate error handling or documentation explaining why this case is now acceptable.

[weswigham]

That's a question for @sandersn - near as I can tell, this branch was added alongside the .FullSignature support, and when enabled, this assertion is pretty immediately triggered in tests. I can't logically see what'd make it unreachable (it is reachable in reality), so away it goes.

[jakebailey]

Even with the build tag a lot of this will end up being executed eagerly; theoretically you could accept a func for these still and then they won't be called in the inlined empty functions so should be eliminated. Not clear how much that matters in this version of debugging though where we only get these in tests.

[weswigham]

Eh, I don't know why this one was a deferred message to begin with - it was just stringifying two kinds, nothing actually expensive. And it's the only assertion (thus far) using the deferred message details pattern, so I just eager'd it rather than adding a deferred-message-taking-overload. In an ideal world under a release tag, the whole fmt.Sprintf should get elided since its' result is unused, anyway. (Weather it actually does, I haven't checked in godbolt, but from what I've read on assertion libs in go, that's the expected codegen for something like this.)

[jakebailey]

It'll still get executed, as far as I know. If this were a pure string concat, that would probably get elided.

[jakebailey]

I suspect the ones in this file should actually be t.Error; it used to be that in Strada we had a couple debug asserts that should have been chai checks but were not.

[weswigham]

There's no t in scope here - it's deep in the test setup logic. It's awkward, at least.

[jakebailey]

Interesting, most FourslashTest take t as their first arg, but I guess we don't have that here.

I'm surprised we don't have it as a plain field at this point. Only one is ever allocated per test. Doing so would simplify all of the methods greatly, so long as we never call a failing T method from the wrong goroutine.

[gabritto]

Sort of meta question, if the assertions won't be enabled in the release build, how are we expected to use them now? Are we to assume that if something goes wrong (and who knows how that'll manifest), some user will report a problem, we will reproduce with a debug-enabled version and see the assertion fail?

[weswigham]

The hope, IMO, is that when you test something you get a repro that fails faster, closer to the root issue - we could also have assertions left on in nightly builds if we wanted, so there'd be a public artifact with assertions on.