[llvm-dev] [cfe-dev] [RFC] Adding support for clang-format making further code modifying changes

Sat Aug 28 12:40:24 PDT 2021

On Sat, Aug 28, 2021 at 5:52 AM Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Sat, Aug 28, 2021 at 3:48 AM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > +1 to what Manuel's said here.
> >
> > One slight change I'd suggest is changing the term "breaking changes" to
> "non-whitespace changes", perhaps? (they aren't necessarily breaking
> anything) At least I assume that's the intent, but I might be wrong in
> which case I'd love to better understand what's being proposed.
>
> To me, the crux of my concern isn't nonwhitespace changes, but changes
> that can make code which used to compile no longer do so. It just so
> happens that nonwhitespace changes are where that risk is highest, but
> whitespace changes can impact syntactic validity (such as reformatting
> preprocessor directives which terminate with an end of line) and
> nonwhitespace changes can (in theory) be written to not break code
> (such as inserting parentheses in expressions such that they match the
> current order of operations used by the expression). So I prefer
> "breaking changes" because it captures the concern better, but I'd
> reword it to "potentially breaking changes" to improve clarity. It's
> not that we expect the option to break significant code (that'd be
> bad), it's that we anticipate that it *could* break code and that's
> why it's treated with greater caution.
>

The language here seems a bit too vague/guarded for my comfort level. Is
the expectation then that someone must demonstrate a specific breakage
situation (however rare/unlikely?) to justify classifying the formatting
feature into this special off-by-default/risky group?

Perhaps we'll end up with easy idioms/obvious proofs that apply to whole
classes (perhaps, for instance, there's an easy/obvious/reusable proof that
applies to most cases of token reordering similar to what Arthur showed?)
of changes & that'll keep the burden of proof fairly low?

In any case, I see the language is intentional, and if the folks working on
this are comfortable with it, I'll leave it to you folks - thanks for
explaining!

- Dave

>
> ~Aaron
>
> >
> > On Fri, Aug 27, 2021 at 7:03 AM Manuel Klimek via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>
> >> On Fri, Aug 27, 2021 at 1:43 PM Aaron Ballman via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>>
> >>> Thanks to everyone who participated in the discussion, and especially
> >>> to MyDeveloperDay for getting this started. Now that discussion on
> >>> this RFC seems to have died down, I think it's worth circling back to
> >>> see if we have consensus to move forward. From reading the threads, I
> >>> think Renato summarized the consensus position nicely:
> >>
> >>
> >> I agree with the following items:
> >>
> >>>
> >>>  * Breaking changes off by default, override behaviour with
> configuration files
> >>>  * All behaviour must be controlled by a configuration flag with
> >>> options explicit on doc/config
> >>>  * Make explicit some options are breaking (comment/naming)
> >>
> >>
> >> I disagree with this one:
> >>
> >>>
> >>>  * Backwards compatibility is pursued, but can be broken in case of
> clear bugs
> >>
> >>
> >> Which I think is also tangential to the current RFC, and IMO should be
> treated in a separate discussion if necessary (this is a fundamental
> problem with how clang-format is designed).
> >>
> >>>
> >>> Unless there is strong opposition to this, then I'd say MyDeveloperDay
> >>> has an answer to their RFC. Any disagreement?
> >>>
> >>> ~Aaron
> >>>
> >>> On Wed, Aug 11, 2021 at 4:37 AM Mehdi AMINI via cfe-dev
> >>> <cfe-dev at lists.llvm.org> wrote:
> >>> >
> >>> >
> >>> >
> >>> > On Tue, Aug 10, 2021 at 5:24 PM Renato Golin via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>> >>
> >>> >> On Tue, 10 Aug 2021 at 15:55, Manuel Klimek via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>> >>>
> >>> >>> I agree that that was probably not obvious or stated very clearly
> anywhere - clang-format for years was basically developed by a small team
> without much interest in input by the rest of the world, for a very
> specific use case.
> >>> >>> At the time we did not do a great job at spreading our thoughts
> with the world, as a lot of discussions were in person.
> >>> >>
> >>> >>
> >>> >> I think there's more to that than what was created by the people
> who created it.
> >>> >>
> >>> >> After being used extensively by people around the world,
> clang-format has perhaps grown to something that the original team did not
> envision.
> >>> >>
> >>> >> However, as a public clang tool, and as part of the LLVM "family",
> it's no longer "a tool used by the team that created it", and not even "a
> tool used by LLVM developers". It is truly a public tool maintained by the
> LLVM community.
> >>> >>
> >>> >> As such, I think we shouldn't restrict the design goals to the
> original design, or to what a small sub-group uses it for. Clang-format is
> an official tool like other visible stuff and should have the same
> community oversight as everything else.
> >>> >>
> >>> >> There is a high demand for a formatting tool that can do so much
> more than the original design and people have been using it, albeit
> precariously, for that purpose already.
> >>> >>
> >>> >> So I believe the original policy that "breaking changes are a
> benefit" can still be valid in a number of places, but it does not (and
> should not) need to be the only valid case. Definitely not the default,
> either.
> >>> >>
> >>> >> I think we need to take a step back and ask users what they expect,
> rather than push forward a policy that people never really wanted but coped
> with it anyway because there's nothing better.
> >>> >>
> >>> >> FWIW, borrowing suggestions from others in this thread, I propose
> we change the policy to:
> >>> >>  * Breaking changes off by default, override behaviour with
> configuration files
> >>> >>  * All behaviour must be controlled by a configuration flag with
> options explicit on doc/config
> >>> >>  * Make explicit some options are breaking (comment/naming)
> >>> >>  * Backwards compatibility is pursued, but can be broken in case of
> clear bugs
> >>> >
> >>> >
> >>> > FWIW, this seems very reasonable to me.
> >>> > (and it is also what I understood MyDeveloperDay's original proposal
> to be).
> >>> >
> >>> > Cheers,
> >>> >
> >>> > --
> >>> > Mehdi
> >>> >
> >>> >
> >>> >>
> >>> >>
> >>> >> An easy way to manage the configuration in this case is to have a
> dump function (`clang-format --dump --config foo.cfg`) that reads a config
> file and dumps all missing parameters and their current values.
> >>> >>
> >>> >> Inline comments would be nice but make configuration management
> hard, so there could be a webpage that describes all options and the URL is
> dumped on the config file.
> >>> >>
> >>> >> cheers,
> >>> >> --renato
> >>> >> _______________________________________________
> >>> >> cfe-dev mailing list
> >>> >> cfe-dev at lists.llvm.org
> >>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >>> >
> >>> > _______________________________________________
> >>> > cfe-dev mailing list
> >>> > cfe-dev at lists.llvm.org
> >>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >>> _______________________________________________
> >>> cfe-dev mailing list
> >>> cfe-dev at lists.llvm.org
> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >>
> >> _______________________________________________
> >> cfe-dev mailing list
> >> cfe-dev at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210828/19c0d134/attachment.html>