I’m curious about this specific error. Since we are adopting the same
.clang-tidy config as upstream MLIR, I don’t know why it would fail for us and not them. I looked into an issue where clang-tidy barfed on our header guard in the
lib/ directory, but didn’t have any issues with upstream MLIR. My hunch is it has to do with some paths hard coded into the checks that don’t work since we are outside the main LLVM source tree. I brought this up last week on the clang-tidy Discord… but haven’t heard back. If there is a way to solve this properly in clang-tidy (instead of NOLINT comments or other hacks), I’m happy to contribute.
More generally about the effectiveness of clang-tidy as an error in the automation: my two cents is if it doesn’t gate a PR from being merged or put a red X next to a commit, no one will look at the warnings. This is my experience from adding these sort of GitHub checks to other projects. Personally, as a newbie to C++ in general and the LLVM/MLIR styles specifically, I appreciate the guardrails. I know @jdd took care to ensure clang-tidy only runs on the code touched by a patch, so failures should only crop up when adding or refactoring code. This helps reduce a unnecessary lint noise while still making sure new contributions are within said guardrails.
I’m generally in favor of keeping clang-tidy in our GitHub workflow as-is, and addressing whatever makes it fail for us but not upstream MLIR. However, if others would prefer to just remove clang-tidy from being a blocker in GitHub, I’m not going to fight it.