Skip to content

Improve has_flags#6824

Merged
waruqi merged 9 commits intodevfrom
opti
Sep 19, 2025
Merged

Improve has_flags#6824
waruqi merged 9 commits intodevfrom
opti

Conversation

@waruqi
Copy link
Member

@waruqi 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消息,不要用中文,请用英文描述。

@waruqi waruqi added this to the v3.0.3 milestone Sep 18, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

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.lua to speed up flag existence checks.
  • Improved Tool Detection: Added opt.norunfile = true to find_cmake.lua and find_ninja.lua to prevent find_program from executing the detected tools, potentially improving performance or avoiding side effects.
  • Reordered Flag Checks: Prioritized checking against the new known_flags cache before performing a full argument list scan in has_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

  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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +49 to +53
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
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

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
end

Since 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"})
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This list of known flags is a good optimization. To make it more comprehensive and improve the effectiveness of this fast path, consider adding other common optimization flags like -O, -Os, and -Ofast.

        known_flags = hashset.from({"-O", "-O0", "-O1", "-O2", "-O3", "-Os", "-Ofast", "-g"})

@waruqi waruqi merged commit 7186aec into dev Sep 19, 2025
43 of 44 checks passed
@waruqi waruqi deleted the opti branch September 19, 2025 00:59
@star-hengxing
Copy link
Contributor

以前 PATH 只有 clang-xx,没有设置 clang alias,使用 --toolchain=clang-xx 是能找到的。
这个 pr 修改后,查找 clang 直接失败而不是 fallback。

可以看到新旧版本顺序。

$ 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!

@waruqi
Copy link
Member Author

waruqi commented Sep 21, 2025

这里面相关的改动就一个。。

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 为什么没找到,就要调下了。

@waruqi
Copy link
Member Author

waruqi commented Sep 21, 2025

再试试,gcc 的也修了

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Try again, gcc's one has also been repaired

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.

3 participants