Can we link clang format into clangAnalysis?

Clang Frontend
1

In the Clang Static Analyzer, we would have a use case for using the clang format library for prettifying macro expansions.
In general, the Static Analyzer community was waiting for this feature and are unanimously supportive.
See the PR156046.

However, we wanted to reach out to the broader Clang frontend area team members to learn their opinion. @clang-area-team

We don’t plan to provide an escape hatch to avoid this link dependency, so everybody building the Clang would be affected. We plan to have a runtime flag to disable invoking the clang format parts (but this code path is anyways only reachable when the Static Analyzer is running, which is a tiny subset of the clang users).

Can we link against clangFormat?

1 Like
2

Clang itself has the same need when formatting fix-its. We currently just say “run clang-format” but it would be better if this was fully integrated into the compiler. Any interest in linking at a higher level than just CSA (e.g., so we can use it from clangParser and clangSema)?

The big question is: what does the library layering look like? clangFormat currently links against:

  clangBasic
  clangLex
  clangToolingCore
  clangToolingInclusions

so we’d have a circular dependency if we tried to do formatting from, say, clangLex.

CC @owenpan @mydeveloperday as clang-format maintainers.

1 Like
3

ping @AaronBallman @owenpan @mydeveloperday

4

I don’t have any problems with this.

5

For the history, the linked PR was merged in Reapply "[clang][analyzer] Format macro expansions" by steakhal · Pull Request #172479 · llvm/llvm-project · GitHub, and released in clang-22.
Under that PR, there was some pushback, so we should have a deeper discussion here.
@thakis

6

Apologies for forgetting this thread existed and not bringing this concern here before.

The community has historically resisted automatically running clang-format on fixes generated by the compiler. This is at least partially due to the fact that clang-format can change the semantic meaning of code depending on the options selected by the user. But there can be other issues as well such as reformatting causing conflicts with other fixes, I believe. Also, the default settings may not match whatever the surrounding source code does (e.g., the user may not be using clang-format for formatting at all), and so on. So our stance has traditionally been that the user is responsible for formatting their code.

That said, there’s also historically been user interest in the fixes we produce not having poor formatting. e.g., users generally prefer consistent whitespace around operators, so if a fix-it produces a +1 instead of a + 1 or a+1, that can be a needless annoyance.

7

I think the dependency on Tooling/Core is arguably the more interesting part. Nothing else in clang used to pull that in. (And it’s a dep of the formatting library iirc.)

The linked PR is for a static analyzer feature. Maybe it could make sense to only have this feature when using the static analyzer via clang-tidy? clang itself is primarily a compiler.

8

@AaronBallman You had a few valid points, especially about the style of the code coming out of clang-format.

We are using this for recover some meaningful formatting for macro expansions for better HTML bug-report visualisation. Think of it a cursor popup bubble of the macro expansion.

Without formatting, due to how we keep track of macro expansions, we will just concatenate all of the expanded tokens joined by spaces. This of course is better than having no expansion help at all, but still looks horrible.

I want feature parity regardless of how the static analyzer is invoked: from clang-tidy or not.
This would simplify the configurations.

I’m open for reverting this, while we figure out a better way integrating this, but then we would need to also revert in clang-22. Otherwise users would get used to it, as it was a long awaited feature.

Shall we do this?

9

Revert PR against llvm/main. Once it lands, I’ll cherry-pick it to release/22.x.