Add clang-tidy and clang-tidy-fix targets

[andreiltd]

This enables clang based linting over c++ codebase. There are two new targets:

• just lint - runs clang-tidy on the codebase
• just lint-fix - runs clang-tidy with -fix option to automatically fix issues

The .clang-tidy file contains minimal configuration to get us started. See enabled Checks.

I was hoping to start the discussion on linting StarlingMonkey in general:

• do we want linting at all,
• should we try to apply the offered fixes,
• should we run the checks on CI,
• should we enforce the checks on PRs?

I hope we can review the errors from the linter and decide which are just the noise and which we want to fix.

cc: @tschneidereit

[tschneidereit]

Some quick responses:

• do we want linting at all,

Yes, I absolutely think we do

• should we try to apply the offered fixes,

I haven't looked at them, but as long as none stick out as very annoying, I'm all for sticking with formatting defaults.

• should we run the checks on CI,

Yes :)

• should we enforce the checks on PRs?

Yes :)

The reason for the last two "yes"s is that that'll help avoid the situation where patches mix functional and formatting changes, making it much harder to identify the former.

[andreiltd]

Thanks! OK, I will move this PR to draft then and try to come up with sensible lint defaults and fixes.

[tschneidereit]

This looks good to me, fwiw. Feel free to un-draft and land this once you feel comfortable with it

[andreiltd]

Hey @tschneidereit, while this PR is rather large, I've tried my best to keep individual commits logically separated. Each commit includes:

• Enabling a specific linter group
• Fixes for the associated lint errors

I've also added a CI step to run the clang-tidy checks automatically but I wonder if I should maybe move this to separate workflow to parallelize the job. The tests are already taking a long time.

[andreiltd]

It'd be great if you could look for those with a regex and fix them in one go.

Maybe it's worth, to just run clang format on the codebase as a very last commit in this PR?

[andreiltd]

I kept my janitor hat on for a little bit longer and cleaned up clang diagnostics for invalid offsetof and disabled the warning globally with the last commit.

While disabling this warning is not ideal, starting from version 20.0, clang introduces the suppression maps option so we can selectively disable the warning only in problematic header files:

https://clang.llvm.org/docs/WarningSuppressionMappings.html

If it's OK, I will add an issue to remove the -Wno-invalid-offsetof compile flag and replace it with suppression mapping when we switch to the next wasi-sdk. WDYT?

[andreiltd]

Hey @tschneidereit, let me know if you wanted to have another pass on this or else maybe we can get this landed?

[tschneidereit]

I wasn't sure whether you were done iterating on it. Will take a look now.

[andreiltd]

I will make a follow up PR where we parallelize the checks.

This change is a bit controversial: https://github.com/bytecodealliance/StarlingMonkey/pull/242#issuecomment-3201119743 so I wanted to double check if you're on board to proceed 🙂

[tschneidereit]

If it's OK, I will add an issue to remove the -Wno-invalid-offsetof compile flag and replace it with suppression mapping when we switch to the next wasi-sdk. WDYT?

Ah, I had been wondering why it was okay to disable these checks. That all makes sense to me, and as long as you/we don't forget to apply that change once we can update SDKs, I'm okay with this plan. Getting rid of all the pragmas sure is nice!