(original) (raw)

I'm all in favor of allowing such changes and will help to create and review
these changes.

Kind regards,
Björn (HazardyKnusperkeks).

Am 10.08.2021 um 10:32 schrieb MyDeveloper Day via cfe-dev:
\> Thanks for the response Sam,
\>
\> Here is how I see we mitigate the risk:
\>
\> On Mon, Aug 9, 2021 at 11:23 PM Sam McCall <sammccall@google.com
\> sammccall@google.com>> wrote:
\>
\> I'm cautiously +1 on const reordering, having previously opposed it and been
\> convinced.
\> 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.
\>
\> \*Risk: \*clang-format will become a grab-bag of features with no clear line -
\> just anything implemented on top of its pseudo-AST.
\> 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).
\>
\>
\> 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.
\>
\> 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,
\> I'm not expecting that to change.
\>
\>
\> \*Risk\*: 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.
\> I think we need to work hard on mitigating this:
\> - the feature needs careful design and extra scrutiny, like
\> security-critical code
\> - it should be clearly and temporarily marked as experimental, with opt-in
\> required
\> - it should be clearly and permanently marked as "makes assumptions about
\> coding style", with opt-in required.
\> - bugs need to be thoughtfully addressed
\> From what I can see MyDeveloperDay is serious about doing all of this.
\>
\>
\> 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)
\>
\>
\> \*Risk\*: 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.
\> 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.
\>
\>
\> 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.
\>
\> 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.
\>
\>
\> 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.
\>
\>
\> 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)
\>
\> 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
\>
\> For example how many times do you see in LLVM the review comment that says
\> "elide the braces" for
\>
\> if (x) {
\> return;
\> }
\>
\> 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.
\>
\> Anyone who knows me, knows I'm very much pro "clang-format all the things"
\>
\> MyDeveloperDay
\>
\>
\>
\>
\>
\> \_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
\> cfe-dev mailing list
\> cfe-dev@lists.llvm.org
\> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
\>

\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
cfe-dev mailing list
cfe-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev