steveire added a comment.
In D69764#2056104 <https://reviews.llvm.org/D69764#2056104>, @rsmith wrote:
> I'm uncomfortable about `clang-format` performing this transformation at all.
> Generally, clang-format only makes changes that are guaranteed to preserve
> the meaning of the source program, and does not make changes that are only
> heuristically likely to be semantics-preserving. But this transformation is
> not behavior-preserving in a variety of cases, such as:
>
> #define INTPTR int *
> const INTPTR a;
> // INTPTR const a; means something else
>
At least in my case, our codebase doesn't have code like that. I wonder how
common it is.
> I also don't think this is something that a user would *want* in
> `clang-format`: changing the location of `const`s is something that would
> likely be done rarely (probably only once) when migrating to a different
> coding style, rather than being done on the fly after each edit to a source
> file.
I don't think this is true.
All of the arguments in favor of `clang-format` existing apply here.
- Developers get it wrong. They put the `{` or the `*` in the "wrong" place
according to the style guide, and they put the `const` in the "wrong" place
according to the style guide.
- `clang-format` is much faster than `clang-tidy` and it is reasonable to have
text editors run it on every "Save" and on all files in a repo on every git
commit etc.
- The above means that the cost of developers getting it wrong is minimized or
eliminated
- The above means that developers don't have to pay attention to `*` placement
or `const` placement while writing code, but can (in theory at least) focus on
what they're trying to convey.
- It seems that more tools understand `clang-format` than `clang-tidy` (eg text
editors with support for it)
> Fundamentally, I don't think this transformation is simply reformatting, and
> I don't think it can be done sufficiently-correctly with only a
> largely-context-free, heuristic-driven C++ parser.
I agree that this is a less simple transformation than existing `clang-format`
functionality.
I can't evaluate whether your macro example (or other examples you could come
up with) mean that it can't be done sufficiently-correctly, but I doubt I would
come to the same conclusion. I would probably base that on whether I could find
real-world code which it breaks, and how common the breaking-patterns (the
macro in your example) actually are in other code.
> As such, I think this belongs in `clang-tidy`, not in `clang-format`.
Given the speed difference and the developer convenience, I think this would be
very unfortunate. I've already tried to write this conversion as a `clang-tidy`
check. However, my implementation has far more bugs than this `clang-format`
implementation, and it does not handle as many cases. You can see that many
`clang-tidy` checks exclude types of code such as "all templates" from
transformation to dampen the complexity of the check implementation.
Besides, a clang-tidy implementation would run into the same problem with your
macro example, wouldn't it? Often the solution to that kind of problem in
`clang-tidy` checks is to simply not transform code in macros. Would an option
in clang-format to not transform around macro code be an more acceptable
solution?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69764/new/
https://reviews.llvm.org/D69764
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
