curdeius added a comment.

I've been trying to make my opinion on this patch for the last few weeks...
I was pretty much opposed to introducing non-whitespace chances previously, but 
I'm reviewing my standpoint.
As mentioned already, there are precedents (include sorting, namespace 
comments, long string splitting).
I'm however even more wary of adding yet another tool that will be almost the 
same as clang-format. It could work if it were a drop-in replacement of 
clang-format, but that seems to be very much wishful thinking to me.
First, maintenance burden to verify that the two don't diverge somehow. 
Secondly, the drop-in replacement wouldn't be possible without changing 
clang-format itself (e.g. to ignore style options that are for "clang-format++" 
only). Also, it might divide the users into clang-format camp and 
clang-format++ camp (which may or may not be a problem).
Lastly, I do think that clang-format can be as reliable with this patch as it's 
now. Breaking code is of course possible but that's the case of many style 
options. And these are bugs that will eventually get fixed. It's of course 
important that this option doesn't break anything ever by default, but given 
that the default is Leave, and it's implemented as an additional pass, that 
should be the case.
Also, I'd be a bit surprised if people used it in CI immediately after this 
feature has landed without verifying that it doesn't break anything on their 
codebase.

On the other hand, clang-tidy has a corresponding check. I do feel though 
that's a sort of heavyweight tool and much less used than clang-format. Also, 
the placing of const qualifier is by many (at least in my circles) associated 
to the latter tool.

So yes, I'm in favour of landing this patch (though not exactly in the current 
form, I'd prefer more future-proof options for instance, not only handling 
const).
My (longish) 2 cents.


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