Skip to content
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

github actions for static code analsys? #590

Closed
jonesmz opened this issue Sep 11, 2022 · 2 comments
Closed

github actions for static code analsys? #590

jonesmz opened this issue Sep 11, 2022 · 2 comments

Comments

@jonesmz
Copy link

jonesmz commented Sep 11, 2022

I have several projects where I've added a github action for

  • clang static analyzer
  • include what you use
  • codeql
  • msvc code analysis

If I create a pull request that added the same, would that be something you'd want to merge?

I ask because all of the current CI is done through appveyor, and I'm not familiar with it, and wouldn't be able to copy-paste (and then modify) my existing analyzers.yml file.

@mosra
Copy link
Owner

mosra commented Oct 10, 2022

Hey, sorry for a late reply on this one.

There's AppVeyor, but there's also CircleCI that I use for all non-Windows builds. (See the full build matrix here.) Problem is that there's aready a lot of combinations, which take a lot of time (or a lot of credits on CircleCI) to go through, so I'd like to add new jobs only if they actually add some value. I'm not opposed to having some builds on GH Actions, relying on multiple providers is never a bad thing, although in my opinion the Actions UX is really terrible compared to CircleCI and AppVeyor. I would also need to write some more javascript to have it appear along the other two on the build status page.

Regarding IWYU, every time I tried it in the past, it was in direct conflict with the forward declaration headers Magnum is using -- i.e., suggesting to remove Trade/Trade.h include and replacing it with manually written forward declarations. Which is annoying and verbose (especially when typedefs are involved) and sometimes even impossible (such as when default template arguments are involved). I'm not aware of the tool fixing this since, and besides that every time I saw it in action it needed a lot of suppressions, "nolint" comments and manual fixups (such as suggesting it should use <utility> instead of <bits/stl_pair.h>, ugh). So that one I definitely don't want.

I remember running Clang Analyzer and LGTM in the past, but they only oever generated false positives for a perfectly valid code, so I eventually ditched them. I also have a stale PR for Clang Memory Sanitizer and UBSan, but that one got also stuck on many false positives that were just too annoying to suppress. In other words, if you do an analyzer build locally and it finds actual bugs (which, based on my past experience, is rather unlikely), I'll consider adding it to the CI build matrix. Otherwise not really.

I have to admit I don't know what CodeQL does. Same as with the analyzers, I can consider adding it if it proves to be useful. If it doesn't or generates far too many annoying / useless warnings like e.g. Clang Tidy, I don't want it.

@mosra
Copy link
Owner

mosra commented Oct 4, 2024

Closing due to the discussion getting stalled.

I'm open to adding an analyzer build (gcc, clang, msvc, whichever) or a fuzz test under the conditions stated above, i.e. that I first see that it found something actually important, and the important thing isn't drowned among a ton of false positives. Without that, it's only increase the (already prohibitively long) total build times.

@mosra mosra closed this as completed Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants