cltest: combine IR and runtime golden checks by luoliwoshang · Pull Request #1771 · goplus/llgo

luoliwoshang · 2026-03-31T07:52:41Z

Summary

add build.Config.ModuleHook so tests can observe a package module before TransformModule, and cover it with a focused unit test
switch the cl compiler suites to cltest.RunAndTestFromDir, so cases with both expect.txt and out.ll can reuse one run/build path while embed target suites can still opt out of IR checks with WithIRCheck(false)
simplify cltest golden handling by sharing one golden reader, keeping IR-only cases on llgen.GenFrom, and capturing mod.String() only for the target package named by the golden ModuleID

Testing

go test ./internal/build -run TestModuleHookReceivesMainPackageModule -count=1
go test ./cl ./ssa ./chore/gentests -run '^$' -count=1
go test ./cl -run 'TestRunAndTestFromTestpy|TestRunAndTestFromTestlibc|TestFilterEmulatorOutput' -count=1

gemini-code-assist

Code Review

This pull request introduces a ModuleHook mechanism to the build configuration, enabling the observation and snapshotting of LLVM modules during the build process. It also adds a new test runner, RunAndTestFromDir, which validates both runtime output and IR snapshots against golden files. The review feedback highlights several performance and maintainability improvements: specifically, making module snapshotting optional to avoid expensive string conversions, using more efficient byte operations when parsing IR files for module IDs, and refactoring the test runners to eliminate code duplication.

gemini-code-assist · 2026-03-31T07:54:51Z

cl/cltest/cltest.go

+	return result.output, nil
+}
+
+func runWithConf(relPkg, pkgDir string, conf *build.Config, usePackagePattern bool) (*runResult, error) {


The runWithConf function now unconditionally stringifies every LLVM module in the build via the ModuleHook. This is an expensive operation that will cause a significant performance regression in the test suite, especially for RunAndCaptureWithConf which is used by many tests that only need execution output. Consider adding a snapshotModules boolean parameter to control this behavior.

References

Identify performance bottlenecks and optimize for efficiency.

gemini-code-assist · 2026-03-31T07:54:51Z

cl/cltest/cltest.go

+	modules := make(map[string]string)
+	prevHook := localConf.ModuleHook
+	localConf.ModuleHook = func(pkgPath string, mod gllvm.Module) {
+		if prevHook != nil {
+			prevHook(pkgPath, mod)
+		}
+		if _, ok := modules[pkgPath]; !ok {
+			modules[pkgPath] = mod.String()
+		}
+	}


Only perform module snapshotting when requested to avoid unnecessary performance overhead. Note that this requires adding the snapshotModules parameter to the function signature and updating callers.

References

Avoid unnecessary calculations or expensive operations.

gemini-code-assist · 2026-03-31T07:54:51Z

cl/cltest/cltest.go

+// RunAndTestFromDir executes tests under relDir and validates both runtime
+// output and the pre-transform package IR snapshot when the corresponding
+// golden files exist.
+func RunAndTestFromDir(t *testing.T, sel, relDir string, ignore []string, opts ...RunOption) {


This function duplicates the directory traversal, ignore set handling, and test selection logic from RunFromDir. Consider refactoring the common logic into a shared helper function to improve maintainability and reduce code duplication.

gemini-code-assist · 2026-03-31T07:54:51Z

cl/cltest/cltest.go

+	lines := strings.Split(string(data), "\n")
+	if len(lines) == 0 {
+		return ""
+	}
+	const prefix = "; ModuleID = '"
+	line := strings.TrimSpace(lines[0])
+	if !strings.HasPrefix(line, prefix) || !strings.HasSuffix(line, "'") {
+		return ""
 	}
-	return output, nil
+	return strings.TrimSuffix(strings.TrimPrefix(line, prefix), "'")


strings.Split(string(data), "\n") is inefficient for large IR files as it copies the entire buffer into a string and then creates a slice for every line. Since only the first line is needed to extract the ModuleID, it's better to use bytes operations to avoid unnecessary allocations.

References

Optimize for efficiency and avoid unnecessary iterations or memory allocations.

xgopilot · 2026-03-31T07:59:21Z

cl/cltest/cltest.go

+		result, err = runWithConf(relPkg, pkgDir, opts.conf, true)
+	} else {
+		conf := build.NewDefaultConf(build.ModeRun)
+		result, err = runWithConf(relPkg, pkgDir, conf, true)
+	}
+	if err != nil && checkOutput {


When checkOutput is false but checkIR is true, a non-nil err from runWithConf is silently ignored and the function proceeds to validate the IR snapshot. If the build partially failed, this could lead to comparing against incomplete/incorrect IR and producing misleading test results. Consider at minimum logging the error, or failing explicitly when the build errors out regardless of which golden files are being checked.

xgopilot · 2026-03-31T07:59:23Z

cl/cltest/cltest.go

+	modules := make(map[string]string)
+	prevHook := localConf.ModuleHook
+	localConf.ModuleHook = func(pkgPath string, mod gllvm.Module) {
+		if prevHook != nil {
+			prevHook(pkgPath, mod)
+		}
+		if _, ok := modules[pkgPath]; !ok {
+			modules[pkgPath] = mod.String()
+		}
+	}


mod.String() serializes the entire LLVM module for every package built, even on the RunAndCaptureWithConf path (usePackagePattern=false) where the captured module strings are never used. Consider gating the hook installation on usePackagePattern (or a dedicated flag) so the existing non-IR code path doesn't pay the serialization cost.

xgopilot · 2026-03-31T07:59:25Z

cl/compile_test.go

+func TestRunAndTestFromTestrt(t *testing.T) {
 	var ignore []string
 	if runtime.GOOS == "linux" {
 		ignore = []string{
 			"./_testrt/asmfull", // Output is macOS-specific.


The original TestFromTestrt called cl.SetDebug(cl.DbgFlagAll) before running IR tests, exercising the compiler's debug-logging paths. The merged test no longer enables debug mode, silently dropping that coverage. If the debug-mode testing was intentional, consider preserving it (e.g., via a WithDebug run option or a separate subtest).

xgopilot · 2026-03-31T08:00:10Z

Good consolidation — merging the separate IR and run test passes into a single build is a solid improvement. The ModuleHook API is clean and well-documented. A few things to address: (1) build errors are silently swallowed when only IR checking is active, (2) mod.String() serialization runs unconditionally even on paths that never use the snapshots, and (3) the cl.SetDebug(DbgFlagAll) coverage from the old TestFromTestrt was dropped in the merge.

codecov · 2026-03-31T08:31:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.87%. Comparing base (d8e5e20) to head (b3bc372).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1771   +/-   ##
=======================================
  Coverage   92.87%   92.87%           
=======================================
  Files          48       48           
  Lines       13353    13353           
=======================================
  Hits        12402    12402           
  Misses        757      757           
  Partials      194      194

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gemini-code-assist bot reviewed Mar 31, 2026

View reviewed changes

cltest: reuse build for IR and run checks

33baf29

xgopilot bot reviewed Mar 31, 2026

View reviewed changes

luoliwoshang force-pushed the codex/pre-transform-module-hook branch from 338fff5 to 33baf29 Compare March 31, 2026 08:01

cltest: keep legacy behavior for partial goldens

e4070a5

luoliwoshang force-pushed the codex/pre-transform-module-hook branch from f02c2fd to e4070a5 Compare March 31, 2026 10:57

luoliwoshang added 3 commits March 31, 2026 21:36

test: simplify cltest helpers

c2090b2

test: simplify cltest run and golden flow

be960b0

test: clean up cltest IR branch

fcbe9fb

luoliwoshang changed the title ~~cltest: reuse build for IR and run checks~~ cltest: combine IR and runtime golden checks Mar 31, 2026

test: capture only target module IR

b3bc372

luoliwoshang force-pushed the codex/pre-transform-module-hook branch from e33ac41 to b3bc372 Compare March 31, 2026 17:31

luoliwoshang requested review from cpunion and xushiwei April 1, 2026 02:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cltest: combine IR and runtime golden checks#1771

cltest: combine IR and runtime golden checks#1771
luoliwoshang wants to merge 6 commits intogoplus:mainfrom
luoliwoshang:codex/pre-transform-module-hook

luoliwoshang commented Mar 31, 2026 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Mar 31, 2026

Uh oh!

gemini-code-assist bot Mar 31, 2026

Uh oh!

gemini-code-assist bot Mar 31, 2026

Uh oh!

gemini-code-assist bot Mar 31, 2026

Uh oh!

xgopilot bot Mar 31, 2026

Uh oh!

xgopilot bot Mar 31, 2026

Uh oh!

xgopilot bot Mar 31, 2026

Uh oh!

xgopilot bot commented Mar 31, 2026

Uh oh!

codecov bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

luoliwoshang commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

xgopilot bot commented Mar 31, 2026

Uh oh!

codecov bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

luoliwoshang commented Mar 31, 2026 •

edited

Loading

codecov bot commented Mar 31, 2026 •

edited

Loading