-
-
Notifications
You must be signed in to change notification settings - Fork 882
feat: Adds support for GCC 15 toolchain #6929
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
Conversation
Summary of ChangesHello @romeoxbm, 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 expands the build system's capabilities by integrating support for the GCC 15 toolchain. This enhancement ensures that the system can properly recognize and utilize GCC 15 for compiling projects, thereby broadening compatibility and offering developers the option to use the latest stable GCC release. Highlights
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this 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 adds support for the GCC 15 toolchain. The approach of creating a new toolchain file that includes the generic GCC configuration is correct and follows the existing project structure.
I've left one comment regarding a potential side effect of using includes, which causes a redundant definition of the generic gcc toolchain. While the fix for this is likely outside the scope of this PR as it would require refactoring shared code, it's an important observation for future improvements to the toolchain architecture.
Additionally, to make GCC 15 support more complete, you might consider updating the feature detection logic in xmake/modules/core/tools/gcc/cxxfeatures.lua to recognize C++23 and C++26 standards, which are supported by GCC 15.
Overall, this is a welcome addition.
| @@ -0,0 +1,3 @@ | |||
| includes(path.join(os.scriptdir(), "../gcc/xmake.lua")) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This includes directive will execute the entire ../gcc/xmake.lua script. That script contains a call to toolchain_gcc() at the end, which defines a generic gcc toolchain. This means that every time this gcc-15 toolchain file is loaded, it will also define the generic gcc toolchain, which is likely an unintended side effect and can be inefficient.
This pattern seems to be inherent in the project's structure, but it's worth pointing out. A potential fix would involve refactoring gcc/xmake.lua to not call toolchain_gcc() when it's being included by other files. This would require changes outside of this PR. As it stands, this could be a source of subtle issues or performance problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is something I already noticed while working on #6899. I may consider addressing it in a separate PR, if my limited spare time allows.
Introduce explicit support for the GCC 15 toolchain.