Conversation
waruqi
commented
Sep 18, 2025
- Before adding new features and new modules, please go to issues to submit the relevant feature description first.
- Write good commit messages and use the same coding conventions as the rest of the project.
- Please commit code to dev branch and we will merge into master branch in feature
- Ensure your edited codes with four spaces instead of TAB.
- 增加新特性和新模块之前,请先到issues提交相关特性说明,经过讨论评估确认后,再进行相应的代码提交,避免做无用工作。
- 编写友好可读的提交信息,并使用与工程代码相同的代码规范,代码请用4个空格字符代替tab缩进。
- 请提交代码到dev分支,如果通过,我们会在特定时间合并到master分支上。
- 为了规范化提交日志的格式,commit消息,不要用中文,请用英文描述。
There was a problem hiding this comment.
Summary of Changes
Hello @waruqi, I'm Gemini Code Assist1! 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 enhances the efficiency of flag detection within the xmake build system, specifically for GCC tools, by implementing a quick-check mechanism for commonly used flags. Additionally, it refines the tool detection process for CMake and Ninja by preventing unnecessary execution of these tools during their discovery, contributing to a more streamlined and potentially faster build environment setup.
Highlights
- Optimized Flag Detection: Introduced a hashset-based cache for common GCC flags (e.g., "-O0", "-g") in
has_flags.luato speed up flag existence checks. - Improved Tool Detection: Added
opt.norunfile = truetofind_cmake.luaandfind_ninja.luato preventfind_programfrom executing the detected tools, potentially improving performance or avoiding side effects. - Reordered Flag Checks: Prioritized checking against the new
known_flagscache before performing a full argument list scan inhas_flags.lua.
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
-
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. ↩
There was a problem hiding this comment.
Code Review
This pull request improves the has_flags check by introducing a fast path for known common flags, which should improve performance. The change to swap the order of checks in main is correct and logical. Additionally, opt.norunfile = true is set when finding cmake and ninja, which is likely an optimization or a fix for cross-compilation scenarios.
I have a couple of suggestions for xmake/modules/core/tools/gcc/has_flags.lua to further improve the implementation: one regarding the caching mechanism to avoid using the global scope, and another to expand the list of known flags for better coverage.
| local known_flags = _g.known_flags | ||
| if known_flags == nil then | ||
| known_flags = hashset.from({"-O0", "-O1", "-O2", "-O3", "-g"}) | ||
| _g.known_flags = known_flags | ||
| end |
There was a problem hiding this comment.
Using the global table _g for caching is not ideal as it can pollute the global namespace and could lead to conflicts. A more robust approach is to use a module-level local variable for caching.
For example, you could declare local known_flags at the top of the file and then initialize it lazily within this function. This would encapsulate the cache within the module's scope.
-- At the top of the file (e.g., after imports)
local known_flags
-- In _check_from_knownargs
if not known_flags then
known_flags = hashset.from({"-O0", "-O1", "-O2", "-O3", "-g"})
end
if known_flags:has(flag) then
return true
endSince this change requires modifications outside of the current diff, I'm providing it as an example here.
| local flag = flags[1] | ||
| local known_flags = _g.known_flags | ||
| if known_flags == nil then | ||
| known_flags = hashset.from({"-O0", "-O1", "-O2", "-O3", "-g"}) |
There was a problem hiding this comment.
|
以前 PATH 只有 clang-xx,没有设置 clang alias,使用 --toolchain=clang-xx 是能找到的。 可以看到新旧版本顺序。 $ xmake f -c -v
checking for platform ... linux
checking for architecture ... x86_64
checking for gcc ... /usr/bin/gcc
checking for nim ... no
checking for nim ... no
checking for clang ... no
checking for clang-20 ... /usr/bin/clang-20
$ xmake f -c -v
checking for platform ... linux
checking for architecture ... x86_64
checking for gcc ... /usr/bin/gcc
checking for nim ... no
checking for clang ... no
error: toolchain("clang-20"): not found! |
|
这里面相关的改动就一个。。 find_xxx.lua 里面如果没找到,就不再 fallback 到通用方案 去 which/where xxx + xxx --version 了,一切都假定 find_xxx.lua 都处理了。 只有不存在 find_xxx.lua 时候,才会 fallback 过去。 这能有效避免很多重复的 which/where.exe 进程执行。在 xxx 找不到的情况下。可以少执行一半的 which/where.exe 所以按理 find_clang.lua 应该得处理一起,包括对 clang-20 的查找。而不是依赖通用的 fallback 逻辑去找。 至于 find_clang.lua 为什么没找到,就要调下了。 |
|
再试试,gcc 的也修了 |
Try again, gcc's one has also been repaired |