Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal: tweak native flags for new toolchain layout #540

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

Young-Flash
Copy link
Collaborator

Copy link

‼️ This code review is generated by a bot. Please verify the content before trusting it.

Here are three potential issues observed in the provided git diff output:

  1. Inconsistent Path Handling for tcc:

    • In the changes, the path to tcc is now explicitly set to $MOON_HOME/bin/internal/tcc. However, this change might introduce issues if the tcc executable is not located in this specific path on all systems. This could lead to runtime errors if the path is not correctly set or if tcc is located elsewhere.
  2. Hardcoded Paths in cc_flags:

    • The cc_flags now include hardcoded paths like -I$MOON_HOME/include and -L$MOON_HOME/lib. This could cause issues if the environment variable $MOON_HOME is not set or points to an incorrect directory. It would be safer to ensure that these paths are dynamically determined or validated at runtime.
  3. Error Handling in set_native_backend_link_flags:

    • The function set_native_backend_link_flags now returns a Result, but the error handling for paths like libmoonbitrun_path is not robust. If libmoonbitrun.o is not found in the expected location, the function will panic due to the use of unwrap(). This should be handled more gracefully to provide meaningful error messages or fallback behavior.

These issues could lead to runtime errors or unexpected behavior, especially in environments where the directory structure or executable locations differ from the expected setup.

@Young-Flash Young-Flash merged commit 2b6d37f into main Dec 31, 2024
5 of 13 checks passed
@Young-Flash Young-Flash deleted the tweak_native_backend_include_path branch December 31, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant