Skip to content

Conversation

@foxg1ove1
Copy link

#6

.gitmodules Outdated
[submodule "rt-thread"]
path = rt-thread
url = https://github.com/RT-Thread/rt-thread.git
url = git@github.com:RT-Thread/rt-thread.git
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件应该不需要修改,依然保留https模式即可

## Project Structure

```
rust/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要对当前目录情况进行update

## Project Structure

```
rust/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要对当前目录情况进行update

@@ -0,0 +1,19 @@
// #include <stdio.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

多看看RT-Thread的编程规范,不应该有这样都是注释掉的代码的,需要遵循对应的风格:

  • 简洁,代码清晰;
  • 代码上不多一行代码,不少一行代码;

@@ -0,0 +1 @@
{"rustc_fingerprint":4312154744837725510,"outputs":{"17747080675513052775":{"success":true,"status":"","code":0,"stdout":"rustc 1.90.0-nightly (4b55fe199 2025-08-01)\nbinary: rustc\ncommit-hash: 4b55fe199cfe9c710555a5af7f2a49491ad38254\ncommit-date: 2025-08-01\nhost: x86_64-unknown-linux-gnu\nrelease: 1.90.0-nightly\nLLVM version: 20.1.8\n","stderr":""},"7971740275564407648":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n/home/fox/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu\noff\npacked\nunpacked\n___\ndebug_assertions\nfmt_debug=\"full\"\noverflow_checks\npanic=\"unwind\"\nproc_macro\nrelocation_model=\"pic\"\ntarget_abi=\"\"\ntarget_arch=\"x86_64\"\ntarget_endian=\"little\"\ntarget_env=\"gnu\"\ntarget_family=\"unix\"\ntarget_feature=\"fxsr\"\ntarget_feature=\"sse\"\ntarget_feature=\"sse2\"\ntarget_feature=\"x87\"\ntarget_has_atomic\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_has_atomic_equal_alignment=\"16\"\ntarget_has_atomic_equal_alignment=\"32\"\ntarget_has_atomic_equal_alignment=\"64\"\ntarget_has_atomic_equal_alignment=\"8\"\ntarget_has_atomic_equal_alignment=\"ptr\"\ntarget_has_atomic_load_store\ntarget_has_atomic_load_store=\"16\"\ntarget_has_atomic_load_store=\"32\"\ntarget_has_atomic_load_store=\"64\"\ntarget_has_atomic_load_store=\"8\"\ntarget_has_atomic_load_store=\"ptr\"\ntarget_has_reliable_f128\ntarget_has_reliable_f16\ntarget_has_reliable_f16_math\ntarget_os=\"linux\"\ntarget_pointer_width=\"64\"\ntarget_thread_local\ntarget_vendor=\"unknown\"\nub_checks\nunix\n","stderr":""},"1001216461566314707":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n/home/fox/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu\noff\n___\ndebug_assertions\nfmt_debug=\"full\"\noverflow_checks\npanic=\"abort\"\nproc_macro\nrelocation_model=\"pic\"\ntarget_abi=\"\"\ntarget_arch=\"riscv64\"\ntarget_endian=\"little\"\ntarget_env=\"gnu\"\ntarget_family=\"unix\"\ntarget_feature=\"a\"\ntarget_feature=\"c\"\ntarget_feature=\"d\"\ntarget_feature=\"f\"\ntarget_feature=\"m\"\ntarget_feature=\"zaamo\"\ntarget_feature=\"zalrsc\"\ntarget_feature=\"zicsr\"\ntarget_feature=\"zifencei\"\ntarget_has_atomic\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_has_atomic_equal_alignment=\"16\"\ntarget_has_atomic_equal_alignment=\"32\"\ntarget_has_atomic_equal_alignment=\"64\"\ntarget_has_atomic_equal_alignment=\"8\"\ntarget_has_atomic_equal_alignment=\"ptr\"\ntarget_has_atomic_load_store\ntarget_has_atomic_load_store=\"16\"\ntarget_has_atomic_load_store=\"32\"\ntarget_has_atomic_load_store=\"64\"\ntarget_has_atomic_load_store=\"8\"\ntarget_has_atomic_load_store=\"ptr\"\ntarget_has_reliable_f128\ntarget_has_reliable_f128_math\ntarget_has_reliable_f16\ntarget_has_reliable_f16_math\ntarget_os=\"linux\"\ntarget_pointer_width=\"64\"\ntarget_thread_local\ntarget_vendor=\"unknown\"\nub_checks\nunix\n","stderr":""}},"successes":{}} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请仔细查看提及的PR,不多一个文件,不少一个文件。这样杂乱无章的代码提交,也是不可能进行合并的。

@foxg1ove1 foxg1ove1 changed the base branch from rust to main October 21, 2025 01:35
@BernardXiong BernardXiong requested a review from Copilot October 24, 2025 17:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive Rust support for RT-Thread on RISC-V 64-bit platforms, enabling embedded Rust development with automatic architecture detection, component management, and example applications.

  • Implements Rust bindings and wrappers for RT-Thread kernel APIs (threads, mutexes, semaphores, queues, filesystem, dynamic loading)
  • Provides automatic target architecture detection for ARM, AArch64, and RISC-V with floating-point ABI support
  • Includes example user applications, components, and dynamic libraries with unified component registration architecture

Reviewed Changes

Copilot reviewed 84 out of 97 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
machines/qemu-virt-riscv64/rust/tools/build_support.py Python build utilities for Rust target detection and cargo integration
machines/qemu-virt-riscv64/rust/src/*.rs Core Rust API wrappers for RT-Thread primitives and system calls
machines/qemu-virt-riscv64/rust/macro-main/src/lib.rs Procedural macro for no_std main function entry point generation
machines/qemu-virt-riscv64/example_usrapp/* Example user applications demonstrating threads, mutexes, queues, filesystem
machines/qemu-virt-riscv64/example_component/* Component registry and logging component with feature-based conditional compilation
machines/qemu-virt-riscv64/rust/Kconfig RT-Thread configuration options for Rust support
Comments suppressed due to low confidence (1)

machines/qemu-virt-riscv64/rust/src/fs.rs:1

  • Incorrect notes text - should describe file operation functionality, not 'micro rust log component'. Consider: 'Rust file operation wrapper'.
/*

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pub fn release(&self) {
semaphore_release(self.sem);
}

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The drop method has incorrect signature and visibility. It should be part of a Drop trait implementation. This method will never be called as a destructor. Replace with impl Drop for Semaphore and make drop lowercase as fn drop(&mut self).

Suggested change
}
impl Drop for Semaphore {

Copilot uses AI. Check for mistakes.

/// Print the last libdl error using RT-Thread `printf`.
pub fn dl_print_last_error() {
// 使用 println! 更符合当前输出宏
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Comment contains mixed language content. For consistency in an English codebase, translate Chinese comment to English: 'Using println! is more appropriate for current output macro'.

Suggested change
// 使用 println! 更符合当前输出宏
// Using println! is more appropriate for current output macro

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@foxg1ove1 代码注释请使用英文。

Comment on lines +152 to +155
Ok(())
} else {
Err(DlError::Close(rc))
}
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition logic is inverted. dlclose returns 0 on success and non-zero on error according to POSIX standard. Should be if rc == 0 { Ok(()) } else { Err(DlError::Close(rc)) }.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@foxg1ove1 这里应该是bug,请进行修正。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// rt-thread/components/libc/posix/libdl/dlclose.c
int dlclose(void *handle)
{
    struct rt_dlmodule *module;

    RT_ASSERT(handle != RT_NULL);

    module = (struct rt_dlmodule *)handle;

    rt_enter_critical();
    module->nref--;
    if (module->nref <= 0)
    {
        rt_exit_critical();

        dlmodule_destroy(module);
    }
    else
    {
        rt_exit_critical();
    }

    return RT_TRUE;
}

// rt-thread/include/rttypes.h
#define RT_TRUE                         1

封装的 dlclose 函数为 pub fn dlclose(handle: *mut c_void) -> c_int; ,所以 rc == 1 为成功而不是 rc == 0

if arg.name.is_none() {
return parse::Error::new(
Span::call_site(),
"`#[marco_main_use]` macro must have attribute `name`",
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in error message: 'marco' should be 'macro'.

Suggested change
"`#[marco_main_use]` macro must have attribute `name`",
"`#[macro_main_use]` macro must have attribute `name`",

Copilot uses AI. Check for mistakes.
Comment on lines 110 to 112
return "riscv32imafc-unknown-none-elf" if ("f" in cflags or "d" in cflags) else "riscv32imac-unknown-none-elf"
if "-march=rv64" in cflags:
if ("-mabi=lp64d" in cflags) or ("-mabi=lp64f" in cflags) or ("f" in cflags) or ("d" in cflags):
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The substring checks 'f' in cflags and 'd' in cflags will match unintended flags like '-ffunction-sections'. Should check for -march= flag parsing instead or use more specific patterns like checking if 'f' or 'd' appears in the march value extracted earlier.

Suggested change
return "riscv32imafc-unknown-none-elf" if ("f" in cflags or "d" in cflags) else "riscv32imac-unknown-none-elf"
if "-march=rv64" in cflags:
if ("-mabi=lp64d" in cflags) or ("-mabi=lp64f" in cflags) or ("f" in cflags) or ("d" in cflags):
march_val = None
mabi_val = None
for flag in cflags.split():
if flag.startswith("-march="):
march_val = flag[len("-march="):]
elif flag.startswith("-mabi="):
mabi_val = flag[len("-mabi="):]
has_f_or_d = False
if march_val and any(x in march_val for x in ("f", "d")):
has_f_or_d = True
if mabi_val and any(x in mabi_val for x in ("f", "d")):
has_f_or_d = True
return "riscv32imafc-unknown-none-elf" if has_f_or_d else "riscv32imac-unknown-none-elf"
if "-march=rv64" in cflags:
march_val = None
mabi_val = None
for flag in cflags.split():
if flag.startswith("-march="):
march_val = flag[len("-march="):]
elif flag.startswith("-mabi="):
mabi_val = flag[len("-mabi="):]
has_f_or_d = False
if mabi_val and (("lp64d" in mabi_val) or ("lp64f" in mabi_val)):
has_f_or_d = True
if march_val and any(x in march_val for x in ("f", "d")):
has_f_or_d = True
if mabi_val and any(x in mabi_val for x in ("f", "d")):
has_f_or_d = True
if has_f_or_d:

Copilot uses AI. Check for mistakes.
@BernardXiong
Copy link
Member

目录结构部分可以参照下 #14 的情况,当前PR的目录结构稍显乱了些。

@BernardXiong BernardXiong requested a review from Copilot October 29, 2025 11:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 89 out of 92 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pub mod libc;
pub mod librt;

pub use librt::*;
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glob re-export from librt at the module level can lead to namespace pollution and makes it unclear which symbols are available. Consider explicitly re-exporting only the needed types/functions or use the glob only in a prelude module.

Suggested change
pub use librt::*;
// Explicitly re-export only the needed items from librt to avoid namespace pollution.
// Example: pub use librt::{item1, item2, item3};
// TODO: Replace with actual items needed from librt.
// pub use librt::{item1, item2};

Copilot uses AI. Check for mistakes.
/// that can be overridden by external code.
#[linkage = "weak"]
#[unsafe(no_mangle)]
fn __rust_panic() -> ! {
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The weak panic handler loops forever without any indication. Consider adding a comment explaining that this is intentional behavior for the default weak implementation, or add a debug output if possible before entering the infinite loop.

Suggested change
fn __rust_panic() -> ! {
fn __rust_panic() -> ! {
// Default weak panic handler: intentionally loops forever to halt execution.
// Override this function for custom panic behavior.
print!("Entered weak panic handler: system will halt.");

Copilot uses AI. Check for mistakes.
*
* Change Logs:
* Date Author notes
* 2025-10-10 foxglove micro rust log component
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change log comment 'micro rust log component' doesn't match the file purpose. This is the filesystem module, not a log component. The comment should describe filesystem operations.

Suggested change
* 2025-10-10 foxglove micro rust log component
* 2025-10-10 foxglove initial filesystem module implementation

Copilot uses AI. Check for mistakes.

```toml
[build]
target = "your_target
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing closing quote. Should be target = \"your_target\".

Copilot uses AI. Check for mistakes.
Comment on lines 45 to 46
# ARM Cortex-M
if has("ARCH_ARM"):
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment '# ARM Cortex-M' is misaligned with the if statement below it. The indentation suggests the comment belongs to the function docstring above, but semantically it describes the if block. Fix the indentation to align the comment with the if statement.

Copilot uses AI. Check for mistakes.
# ABI must carry f/d to actually use hard-float calling convention
abi = info["mabi"] or ""
abi_has_fp = abi.endswith("f") or abi.endswith("d")
has_fpu = has("ARCH_RISCV_FPU") or has("ENABLE_FPU") or info["has_f"] or info["has_d"]
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable has_fpu is not used.

Copilot uses AI. Check for mistakes.

# List all files in the directory for debugging
try:
files = os.listdir(search_path)
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable files is not used.

Copilot uses AI. Check for mistakes.
.gitmodules Outdated
[submodule "rt-thread"]
path = rt-thread
url = https://github.com/RT-Thread/rt-thread.git
url = git@github.com:RT-Thread/rt-thread.git
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里可以不用修改的。

@@ -1,1448 +0,0 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件为什么会被删除掉?

Comment on lines +152 to +155
Ok(())
} else {
Err(DlError::Close(rc))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@foxg1ove1 这里应该是bug,请进行修正。


/// Print the last libdl error using RT-Thread `printf`.
pub fn dl_print_last_error() {
// 使用 println! 更符合当前输出宏
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@foxg1ove1 代码注释请使用英文。

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.

2 participants