<div dir="ltr"><div>Thanks for the response Sam, </div><div><br></div><div>Here is how I see we mitigate the risk:</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 9, 2021 at 11:23 PM Sam McCall <<a href="mailto:sammccall@google.com">sammccall@google.com</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"><div dir="ltr"><div>I'm cautiously +1 on const reordering, having previously opposed it and been convinced.</div><div>I think anyone who's worked on a large shared codebase both before and after clang-format can understand the value here, so I'll focus mostly on the risks and why I think they're acceptable.</div><div><br></div><div><b>Risk: </b>clang-format will become a grab-bag of features with no clear line - just anything implemented on top of its pseudo-AST.</div><div>Clang-format's brand is low-level formatting details and I think it's important to preserve this. Const order fits here in users' minds. (So does brace addition/removal).</div></div></blockquote><div><br></div><div>I doubt we wouldn't continue to apply the same level of scrutiny on the code reviews and expect them to follow best practices and guidelines, I am expecting us to still be quite circumspect as to what we'd consider.</div><div><br></div><div>To be honest clang-format I think already runs at quite a high review rejection rate, people ask for all sorts of things and we do try to push back pretty hard, landing something can sometimes be pretty torturous to get through review,</div><div>I'm not expecting that to change.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div><b>Risk</b>: The feature will break code and clang-format will no longer be (seen as) reliable. This can make it harder socially or technically to deploy, and cause real damage.</div><div>I think we need to work hard on mitigating this:</div><div> - the feature needs careful design and extra scrutiny, like security-critical code</div><div> - it should be clearly and temporarily marked as experimental, with opt-in required</div><div> - it should be clearly and permanently marked as "makes assumptions about coding style", with opt-in required.</div><div> - bugs need to be thoughtfully addressed</div><div>From what I can see MyDeveloperDay is serious about doing all of this.</div></div></blockquote><div><br></div><div>I am, I also think that we shouldn't plough on with individual changes if we see them as potentially ambiguous, I would rather ignore a change if in doubt, I don't feel such features need to be 100% catch all (like how sometimes clang doesn't always tell you about all missing overrides, just as it can rationalize them), This may require more specific options to ensure we know what an tok::identifier actually is in order to avoid ambiguities caused by macros (a little like StatementMacros)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div><b>Risk</b>: clang-format will be overtaken by the complexity of such features, which will outweigh the benefits (if few use them), hurting maintenance, causing bugs etc.</div><div>However this isn't different from other optional features. Editing tokens tends to be done as a separate pass which is relatively easy to isolate (compared to something like supporting a new language). With complexity isolated, this is mostly just about how maintainers prioritize their time/attention, which must be left up to them.</div></div></blockquote><div><br></div><div>To be honest these are likely some of the less invasive features (in comparison to say adding something like adding Whitesmiths style or C#), as you say the "Passes" give us an easy mechanisms to handle the "OptIn" without adding "if (...) everywhere and the passes also tend to be very self contained especially as the Formatting itself is just a Pass in its own right which is performed later.</div><div><br></div><div>I have no concerns over the maintenance other than ensuring we understand how new passes actually work, but the compartmentalization feels on a par to 

compartmentalization of individual clang-tidy checks.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Regarding include-ordering: I think this is a valuable feature if you follow a coding style that allows it to be correct, and it fits well in clang-format's brand. However it wasn't clearly labeled to emphasize its caveats, and in hindsight it shouldn't have been made part of the Google style without further opt-in required.</div><div><br></div></div></blockquote><div><br></div><div>To be honest as a developer I like the brutality of include-ordering, breaking my code only tells me it isn't robust enough (likely missing forward declarations or not including what its using) </div><div><br></div><div>The handling of defaults is always difficult as some people want things and others don't, (hence the need for the RFC), but I've always been clear this needs to be "Opt-In" from the start. For the majority of  developers I would expect them to continue to use clang-format as a code formatter and nothing else, but having a ability to make some (not all) obvious changes could potentially be a great help to improving code</div><div><br></div><div>For example how many times do you see in LLVM the review comment that  says "elide the braces" for </div><div><br></div><div>if (x) {</div><div>    return;</div><div>}</div><div><br></div><div>I feel this is something that clang-format could be made to easily handle. This RFC is about gaining a general consensus to let us try. We feel we can add even more value.</div><div><br></div><div>Anyone who knows me, knows I'm very much pro "clang-format all the things"</div><div><br></div><div>MyDeveloperDay</div><div><br></div><div><br></div><div><br></div><div><br></div><div> </div></div></div>