Delete GCC static analysis CI

[ISSOtm]

EDIT: scope was changed after #1950 (comment)

---

This does raise at least one warning, which thus fails the build. That does goes on to show that we weren't paying attention to them before!

[Rangi42]

I actually do occasionally check the analysis workflow! But as you can see, this is a new warning now that the analysis runs on Ubuntu 22.04, not just ubuntu-latest. (Nice coverage improvement!)

[Rangi42]

Suggested change

[Rangi42]

Suggested change

	# It is also very expensive, both in terms of CPU and RAM.
	# It is also very expensive, both in terms of CPU and RAM, so mark it as advanced.

[ISSOtm]

That comment was related to why it's not enabled by default as part of our develop profile; the rationale for marking it as “advanced” is that downstream users shouldn't need to care about it. (I think we should do the same with e.g. MORE_WARNINGS, but I'm still thinking about it and thus saving it for another PR.)

[Rangi42]

This comment being right above the mark_as_advanced line, instead of right after the "Note that" sentence comment (which would be the natural way to read it, as one sentence continuing another) made me think that it was here for a reason, to describe the subsequent line. If "very expensive" is justifying the OFF, it should definitely be above the the with the OFF in it.

(And I don't really understand why to bother using mark_as_advanced anyway; doesn't that just inconveniently hide it from users who didn't already know to opt in to "advanced" mode when checking their GUI's option flags list? It's a bit like needing to pass man foobar --advanced to actually see how foobar works.)

[ISSOtm]

Well, I don't know of any official guidance on what to mark as Advanced, but I think it's useful to treat it like CMake's -Wdev versus -Wno-dev: non-advanced things are intended to be usable by anyone using our build system, whereas “advanced” toggles are maintainer-oriented instead. Which thus makes sense to hide unless it's opted into.

[ISSOtm]

Note also that this comment is below the mark_as_advanced line.

[Rangi42]

Okay, so the new analysis error is:

src/cli.cpp:83:16: error: dereference of NULL ‘__dest’ [CWE-476] [-Werror=analyzer-null-dereference]

This is the relevant code:

    83          struct AtFileStackEntry {
    84                  int parentInd;            // Saved offset into parent argv
    85                  std::vector<char *> argv; // This context's arg pointer vec
    86
    87                  AtFileStackEntry(int parentInd_, std::vector<char *> argv_)
    88                      : parentInd(parentInd_), argv(argv_) {}
    89          };

I'm pretty sure __dest is some internal field of the std::vector<char *> argv;, and that this is another false positive.

Actually, gcc's current docs say:

The analyzer is only suitable for use on C code in this release.

@ISSOtm So maybe we should just not be running this expensive CI step at all.

[ISSOtm]

At this point, I'm questioning whether we want to have any such support at all. How about I turn this PR into just deleting analysis.yml?

[Rangi42]

At this point, I'm questioning whether we want to have any such support at all. How about I turn this PR into just deleting analysis.yml?

Sounds fine to me.

Scope's been pivoted