runtime/internal/clite/syscall: remove deps errors

[visualfc]

runtime/internal/clite/syscall

• remove deps errors
• remove error
• remove Errno methods

[gemini-code-assist]

Summary of Changes

Hello @visualfc, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the error handling mechanism within the runtime/internal/clite/syscall package and its consumers. The primary goal is to decouple the syscall package from directly instantiating error types by defining error messages as simple string constants. This change centralizes the canonical text for common errors, while allowing higher-level packages like oserror to construct error values from these strings, thereby maintaining consistent error messaging and potentially simplifying dependencies.

Highlights

• Error Definition Refactoring: The runtime/internal/clite/syscall package no longer directly creates error instances. Instead, common error messages like ErrInvalid and ErrPermission are now defined as string constants.
• Error Comparison Logic Update: The Errno.Is methods in syscall_unix.go and syscall_wasm.go have been updated to compare the string representation of an error (target.Error()) against the new string constants, adapting to the change in error definition.
• Error Re-instantiation in oserror: The runtime/internal/lib/internal/oserror package now explicitly re-creates standard error types by wrapping the syscall package's string constants using errors.New(), ensuring that consumers of oserror still receive proper error values.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the definition of common syscall errors, changing them from error variables to string constants. This is done to remove the errors package dependency from runtime/internal/clite/syscall. While the goal of reducing dependencies in low-level packages is understandable, the approach taken has significant drawbacks.

The core issue is that this change forces error checking to rely on string comparison (err.Error() == ...), which is a fragile and non-idiomatic pattern in Go. It makes the system brittle, as any future change to an error message will silently break error handling logic.

My review includes critical feedback on this architectural change, recommending a reversion to the standard practice of using sentinel error variables. This will restore the robustness and maintainability of the error handling code.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.00%. Comparing base (34b872c) to head (f43a8c4).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1543   +/-   ##
=======================================
  Coverage   91.00%   91.00%           
=======================================
  Files          45       45           
  Lines       11955    11955           
=======================================
  Hits        10880    10880           
  Misses        899      899           
  Partials      176      176           

☔ 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.

[visualfc]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors error handling in the syscall packages by changing many function signatures to return Errno instead of error. This is a good step towards centralizing error message generation and decoupling the low-level clite/syscall package. However, I've found a few critical issues that will cause compilation failures due to incorrect return values, and a couple of high-severity logic bugs related to error handling and unreachable code. Please address these points to ensure the correctness of the changes.

[visualfc]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the syscall-related packages to remove dependencies and change the error handling strategy. The core change is that functions in runtime/internal/clite/syscall now return a numeric Errno instead of an error, which is a good practice for low-level packages to avoid allocations. The runtime/internal/lib/syscall package is updated to act as an adapter, converting these Errno values back into error types for higher-level consumers. The sentinel errors have also been moved to runtime/internal/lib/internal/oserror to break dependency cycles.

The changes are well-executed and consistent with the stated goal. I've found a couple of potential oversights related to this large refactoring:

• The ErrUnsupported error seems to have been dropped during the move of sentinel errors.
• The wasm implementation of lib/syscall.Errno is missing the Temporary() and Timeout() methods, leading to an API inconsistency with the unix version.

My detailed comments address these points. Overall, this is a good refactoring that improves the architecture of the syscall handling.

[visualfc]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the syscall error handling by moving logic from the runtime/internal/clite/syscall package to the runtime/internal/lib/syscall and runtime/internal/lib/internal/oserror packages. The clite/syscall.Errno type no longer implements the error interface, making it a more primitive, C-like error code. The lib/syscall package now provides the Go error interface wrapper. This change is applied consistently across unix and wasm builds, and dependent packages are updated accordingly. The changes improve the layering and separation of concerns. The implementation appears correct and thorough.

[visualfc]

/gemini summary

[gemini-code-assist]

Summary of Changes

This pull request refactors the error handling mechanism within the runtime/internal/clite/syscall package by removing its direct dependency on the standard errors package and error interface methods for its Errno type. Instead, Errno is now returned directly by syscall functions, and error string representations are handled by a global function. Dependent packages have been updated to align with this new approach, either by re-establishing standard error types or by re-implementing Errno interface methods to maintain compatibility with Go's error handling patterns.

Highlights

• Centralization of Error String Definitions: The runtime/internal/clite/syscall package now defines error messages as simple string constants rather than error instances, aiming to centralize canonical error texts.
• Decoupling of Errno from error interface methods: The Errno types in runtime/internal/clite/syscall no longer implement Error(), Is(), Temporary(), or Timeout() methods directly. Instead, a global Error(e Errno) function is provided for string representation.
• Migration to Errno Return Types: Several functions within runtime/internal/clite/syscall and syscall_wasm.go that previously returned error now return Errno directly.
• Adaptation in Dependent Packages: Packages like runtime/internal/lib/internal/oserror and runtime/internal/lib/syscall have been updated to accommodate these changes, either by redefining errors using errors.New() or by re-implementing Errno methods to maintain compatibility with Go's error handling patterns.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• runtime/internal/clite/syscall/syscall.go
  • Removed import of errorsPkg.
  • Deleted var declarations for common errors (e.g., ErrInvalid, ErrPermission).
  • Modified the Kill function's signature to return Errno instead of error, and changed its success return value from nil to 0.
• runtime/internal/clite/syscall/syscall_unix.go
  • Refactored the Error() method of Errno from a method to a package-level function Error(e Errno) string.
  • Removed Is(), Temporary(), and Timeout() methods from the Errno type.
• runtime/internal/clite/syscall/syscall_wasm.go
  • Refactored the Error() method of Errno from a method to a package-level function Error(e Errno) string.
  • Removed Is(), Temporary(), and Timeout() methods from the Errno type.
  • Updated the return types of Sysctl, Getgroups, Sendfile, StartProcess, Wait4, SetNonblock, and Getrlimit functions from error to Errno, adjusting their success return values from nil to 0.
  • The commented-out Gettimeofday function signature was also updated to reflect an Errno return type.
• runtime/internal/clite/syscall/tables_wasm.go
  • Removed the errEAGAIN, errEINVAL, errENOENT variables and the errnoErr helper function, which were used for boxing common Errno values into error interfaces.
• runtime/internal/lib/internal/oserror/errors.go
  • Changed the import from github.com/goplus/llgo/runtime/internal/clite/syscall to the standard errors package.
  • Redefined ErrInvalid, ErrPermission, ErrExist, ErrNotExist, and ErrClosed directly using errors.New() from the standard errors package.
• runtime/internal/lib/os/dir.go
  • Adjusted the error return in ReadDir from syscall.Errno(os.Errno()) to origSyscall.Errno(os.Errno()) to align with the refactored syscall package.
• runtime/internal/lib/syscall/syscall.go
  • Modified the Kill function to correctly handle the Errno return from the underlying syscall.Kill call, converting 0 to nil and other Errno values to the package's Errno type.
• runtime/internal/lib/syscall/syscall_unix.go
  • Removed imports for unsafe and c "github.com/goplus/llgo/runtime/internal/clite", and added an import for the standard errors package.
  • Updated the Errno.Error() method to delegate to the new syscall.Error function.
  • Expanded the Errno.Is() method to include a case for errors.ErrUnsupported.
• runtime/internal/lib/syscall/syscall_wasm.go
  • Added imports for the standard errors package and github.com/goplus/llgo/runtime/internal/lib/internal/oserror.
  • Updated the Errno.Error() method to delegate to the new syscall.Error function.
  • Re-implemented the Errno.Is(), Errno.Temporary(), and Errno.Timeout() methods for consistency with the unix counterpart and to adapt to the new syscall.Errno type.

Activity

• An initial summary of changes was provided by gemini-code-assist[bot].
• A code coverage report was posted by codecov[bot], indicating 91.00% project coverage and all modified lines covered.
• The author, visualfc, invoked /gemini review or /gemini summary multiple times.
• Critical review comments were provided by gemini-code-assist[bot] regarding the error handling refactoring, specifically: issues with changing sentinel errors to string constants, anti-patterns in Errno.Is method, nil vs 0 return values for Errno, unreachable code, incorrect Kill function error handling, and missing Temporary()/Timeout() methods and ErrUnsupported error in syscall_wasm.go.