aaron.ballman 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
>


That's an excellent point. I agree that the formatting cannot change the 
meaning of the code that's being formatted -- that would be bad.

> 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'm not certain I agree with this. For instance, you can run clang-format as a 
precommit step or as part of your CI pipeline to bark at users when they get 
formatting incorrect while working on a team project. (We do this internally 
with some of our internal projects.)

> 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. As such, I think this 
> belongs in `clang-tidy`, not in `clang-format`.

I think clang-tidy has the machinery needed to do this properly, but I think 
clang-format is logically where I would go to look for the functionality 
(certainly before thinking of clang-tidy) because this is more related to 
formatting than not. That said, if we cannot make it work reliably within 
clang-format, I'd rather see it in clang-tidy than nowhere.


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

Reply via email to