Clang tidy integration

[parsifal-47]

I know, it may look invasive, but do not hesitate to ask for changes, I tried to exclude some policies that are often violated in the code like braces-around-statements or the ones that are producing false alarms.

Test plugin does not fix errors, it only checks for compliance.

Let me know what you think and feel free to ask questions!

[red1bluelost]

What was the reason that this one had to change? It seems like we expect lhsValue to be non-null in the else case (same with rhsValue). I think a better change here would be to not initialize lhsValue at the declaration and instead initialize it in each branch. Like the following:

Value lhsValue;
if (...) {
  ...
  lhsValue = lhsOp.getResult();
} else {
  lhsValue = cast<Value>(lhs);
  assert(...);
}

Make sure to use cast and not dyn_cast. Do the same with rhsValue.

[parsifal-47]

The reason for this change is, Null pointer deference is Undefined behavior. The code you suggesting will behave the same, cast will crash if lhs cannot be casted to Value. But I agree it is better, updated, thank you!

[nhat-nguyen]

Thank you @parsifal-47! I took a brief look and there are a few style changes that are a bit unfamiliar to myself:

• unused arguments don't have names but instead use /* name */
• auto * for pointers

I don't know how common these patterns are, but I think we should probably keep these intact. @red1bluelost Any thoughts?

Another unrelated question: we use clang-format with the llvm style internally but not clang-tidy. What is the main difference between clang-tidy and clang-format? Is it common to use both?

Also I think it would be good to have a github action that automatically checks for style violations.

[parsifal-47]

Another unrelated question: we use clang-format with the llvm style internally but not clang-tidy. What is the main difference between clang-tidy and clang-format? Is it common to use both?

clang-format has no static analysis. It doesn't use clang parser, it uses its own lightweight parser which is optimized for formatting, it results in different reading of the code and some engineers do not like it. I was using it multiple projects and can set it up here.

Also I think it would be good to have a github action that automatically checks for style violations.

Yes, I agree, unless the style is checked, it won't be followed. Let me set it up.
Thank you!