<div dir="ltr"><div>+1 for not only handling "const". I've often tried getting the various bits that appertain to a declaration (static const volatile constexpr inline consteval) sorted in a consistent order - that makes them much more greppable.</div><div><br></div><div>Different patch, I expect, though.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 14, 2021 at 1:47 PM Marek Kurdej via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">curdeius added a comment.<br>
<br>
I've been trying to make my opinion on this patch for the last few weeks...<br>
I was pretty much opposed to introducing non-whitespace chances previously, but I'm reviewing my standpoint.<br>
As mentioned already, there are precedents (include sorting, namespace comments, long string splitting).<br>
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.<br>
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).<br>
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.<br>
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.<br>
<br>
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.<br>
<br>
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).<br>
My (longish) 2 cents.<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D69764/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D69764/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D69764" rel="noreferrer" target="_blank">https://reviews.llvm.org/D69764</a><br>
<br>
</blockquote></div>