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

David Blaikie via cfe-dev cfe-dev at lists.llvm.org
Mon Aug 30 10:02:54 PDT 2021 

On Sun, Aug 29, 2021 at 6:46 AM Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Sat, Aug 28, 2021 at 3:40 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > 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?
>
> Sort of? My thinking is: if someone comes up with a test case that the
> clang-format developers do not consider to be a bug (and thus don't
> intend to "fix" the behavior), then we absolutely should document it
> as described. (Also, I think it should be an explicit test case
> showing the behavior with a comment about it being expected behavior
> -- that helps with communication if someone files a bug about it.) If
> no one can come up with a test case that would break, I'd be fine if
> we still classified it as a potentially breaking change based on
> whitespace vs not whitespace or some other metric, but my bigger point
> is that if someone can demonstrate a test case that break that isn't
> sufficiently compelling to change the tool to handle better, that
> definitely meets the bar for being opt-in.
>

Yeah, this is the vague/uncertain aspects I'm a bit concerned with: If
someone comes along with a test case demonstrating a formatting rules can
break certain code and folks classify it as effectively "will not fix
because it's not worth it/diminishing returns" we now have to change the
rule to be off-by-default (where it was possibly on-by-default before). But
then if someone decides to fix that bug, because they have a use-case that
makes it worthwhile for them to fix it - now we might flip it back to
on-by-default?

If that's unlikely - if most of the time the answers are clear, that the
breakage is /super/ expensive to avoid such that no one's likely to ever
revisit the decision/have a different cost/benefit tradeoff - and the
breakages are obvious/easy to demonstrate (like I said, if there's common
types of breakage and so simple breakage patterns that can be used in most
cases to demonstrate breakage, etc) then we might avoid these sort of
flip/flop cases most of the time.


>
> > 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?
>
> That seems plausible.
>
> > 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!
>
> Any time, thanks for the discussion!
>
> ~Aaron
>
> >
> > - 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/cfe-dev/attachments/20210830/463c3ce8/attachment-0001.html>

More information about the cfe-dev mailing list