rsmith added a comment.

In D69764#2057945 <https://reviews.llvm.org/D69764#2057945>, @aaron.ballman 
wrote:

> In D69764#2056104 <https://reviews.llvm.org/D69764#2056104>, @rsmith wrote:
>
> > 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.)


You can do the same with clang-tidy checks. If you're running clang-format but 
not clang-tidy as part of your CI, it would likely be worth your while to add 
clang-tidy to your set of CI checks regardless of what we do here. But if you 
want this only as a CI check, and not for reformatting code as you edit it, 
then for me that is a fairly strong signal that this does not belong in 
clang-format.

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

Right now we have a relatively clear line between the tools. clang-format does 
not parse or really understand the code, and just heuristically puts the 
whitespace and line breaks in the right place, in a way that is ~always 
correct. clang-tidy understands the meaning of the program and can suggest 
changes that are likely correct (but should typically be double-checked by a 
person). I think this kind of change is in the latter category.

I totally agree that it's reasonable to think of this as a reformatting change, 
just as I think it's reasonable to think of (say) reordering the data members 
of a class to the start as a reformatting change, or to think of moving an 
inline function definition out of the class definition as a reformatting 
change, or parenthesizing certain operators as a reformatting change -- and all 
of those could also reasonably be required by some house coding style. But I 
don't think clang-format is the right tool to perform those operations.


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