-
Notifications
You must be signed in to change notification settings - Fork 141
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
A lot of warnings from GCC 14.2 #1818
Comments
The |
The inline/noinline collisions and other attribute issues seem at first glance like things that should legitimately be fixed, but I haven't looked at them in detail yet. In any case, generally, warning and compiler settings are something I am paying attention to and are likely to evolve. Generally gcc diagnostic flags are less kept up than clang and MSVC ones at present. |
Regarding the YMFM warnings, the only place we call that timer function is within a for loop with a range of 2, meaning we only pass 0 or 1. Most likely scenario is the compiler is having trouble detecting that the only values passed at 0 or 1 due to template shenanigans in nall. |
There is a related issue #1399 which I raised some time ago. I'm interested in fixing some of these warnings, if I can. Would the project accept a pull-request which:
|
I would recommend against this; compiler strictness is still a moving target in the new build system, and PRs will already show inline warning annotations (though only from clang). |
Doesn't have to be a moving target, a well-configured workflow could reject specific warnings. Here is something I do in my day job: I use clang-tidy to make sure that the core guidelines I care about are enforced. Many other lints are of course ignored. Something like this is my .clang-tidy configuration:
I might also enable new lints as and when the code improves |
Moving target might have been the slightly wrong term to use. My point is more so that integrating increased code strictness and code quality tools is a work in progress at the moment, and that that work has to be undertaken carefully and methodically, with an eye toward what is appropriate for ares in particular, given its idiosyncratic codebase and code style as well as the need to continually consider the wide array of toolchains ares supports. My top priority at the moment is giving us the ability to effectively isolate diagnostic flags in nall from the rest of our targets (we cannot do this at the moment because every target is effectively also compiling nall). From there, I want to work on the clang diagnostic flags (and potentially other toolchain diagnostic flags) in some targets to address the things that seem to have the highest probability of being actual bugs (as well as any issues uncovered by the clang analyzer, though we are mostly ok on this front). These are mostly things like suspicious implicit conversions, potential aliasing issues, unused warnings, signed/unsigned comparisons, and generally the extent to which ares plays fast and loose with types. Many of these issues do overlap with checks that exist in clang-tidy, just occurring at compile time. Linting tools like clang-tidy might have value for ares, but at the moment I consider them less of a priority (other than the analyzer and the overlap with actual compiler diagnostics) because of their being more challenging to integrate with ares and having more of a style focus. As mentioned, ares has an idiosyncratic style, but that style is well-established and developers are well-acquainted with it. Unless the tool is identifying things that have a high probability of being genuine application bugs, rather than style differences between ares and LLVM (and admittedly this can be something of a grey area), I think there may be more important fish that need frying first. Regardless, I would definitely welcome experimentation with its output to see if it can deliver useful feedback not already provided by the analyzer and clang itself! Though you didn't mention it directly, a word on formatting as well: I think ideally, codifying ares's code style into something understood by clang-format would be nice, it would also be challenging to integrate across the codebase for various reasons, and introduce more complexity. I think the focus should be on what problem the enforced formatting would solve. At the moment, ares's style is pretty uniform, and radically different styles coming from different contributors, or in different areas of the codebase, does not seem to be an issue at present. As such, I do not see a pressing need to introduce enforced formatting, though in the long term it may be a good idea. |
Note that when I reported the warnings to ymfm, he basically said it's a problem on our end
The text was updated successfully, but these errors were encountered: