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

Tue Aug 10 12:10:08 PDT 2021

On Tue, Aug 10, 2021 at 12:02 PM Chris Lattner via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> First of all, thank you so much for writing up this clear and mostly
> balanced summary of the situation, the concerns, and the decision point
> ahead here.
>
> I think there is a something fundamental problem to the nature of this
> discussion: we can all make semi-informed guesses about how well a
> particular format will work in practice, but we can’t know until there is
> usage experience.  Furthermore, clang-format has multiple different
> communities with different tradeoffs, and imposing a new format on an
> existing codebase has concerns.
>
> Clang format is already highly parameterized to support this, are you
> saying that there is pushback on adding new off-by-default capabilities to
> clang-format, or is the pushback about adding these capabilities to
> existing language modes (llvm style, google style, etc)?  I don’t see an
> obvious problem with introducing off-by-default capabilities.
>
> I can see a reasonable concern about introducing “const moving” or other
> new things into existing formats by default - that could be disruptive, and
> depends a lot about how well the algorithm and implementation works in
> practice.  Two thoughts on how to make progress here:
>
> 1) You could implement these things in an off-by-default setting, ship it
> out to lots of people, then gain data somehow (e.g. ask for feedback).
> 2) We could introduce a new top level “changing my code is allowed” mode
> to clang-format and put these checks into that.  You could even
> conceptually move namespace commenting and include sorting to that mode,
> making clang-format more consistent.
>

I don't think anyone's suggesting these things would be on-by-default. I
believe the discussion/debate was about adding the functionality (off by
default) at all. It's certainly been a discussion and an angle
("clang-format must not make more-than-whitespace changes, generally" (with
some carveouts like include sorting)) - which, apparently was an overly
strong statement compared to the original design intent (not sure where I
got the impression that "nothing but whitespace" was a core design
principle, but a bunch of us did - which made it mostly a reality for quite
a while).

but sounds like most folks are happy for "more than whitespace changes are
OK so long as they're off by default" (but could be on by default under
specific clang-format configurations like Google and LLVM) which I think is
OK for those asking for the features & seems mostly OK for folks who don't
want the features (some folks like Andrew and Aaron who are advocating for
a stronger separation - such as non-whitespace changes only being
accessible under a separate tool name).

- Dave

>
> -Chris
>
> On Aug 9, 2021, at 12:36 PM, MyDeveloper Day via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
> Hi all,
>
> As a frequent user and maintainer of clang-format I would like to
> formalize a change to the "charter" of what clang-format can/could/should do
>
> Motivation
> ==========
>
> As you all know clang-format parses source code and can manipulate the
> tokens to produce consistently formatted code (most of the time), its
> become ubiquitous in the industry and its integration into
> popular editors & IDEs such as vim/visual studio/code mean it very simple
> for users to get setup and running producing good looking code.
>
> clang-format does not use semantic information, and as such it doesn't
> need includes, defines or compiler flags to interpret code. Clang-format is
> generally guessing that certain sequences of tokens from the lexer
> represent certain patterns. It's a good guess but it gets it wrong from
> time to time, hence trading correctness for performance.
>
> Because of this clang-format is fast (not maybe as fast as we'd like) but
> fast enough to be part of in a "save" operation such that the code is
> formatted as the ide saves your work.
>
> Mostly clang-format manipulates only whitespace, but over the years there
> have been a number of extremely useful features that have broken this rule,
> namely include sorting, namespace commenting to name a few.
>
> The usage scenario of clang-format has also changed slightly to include a
> non modifying advisory role identifying clang-format violations (as in our
> own llvm-premerge checks), which can greatly aid the code review process by
> removing the need to constantly ask for the code to be formatted correctly
> or follow the LLVM convention.
>
> Recently a number of new features have been proposed that would also alter
> code, insertion of braces, east/west const conversion that can be performed
> at "save" speeds.
>
> As no semantic information is present, this raises the question as to
> whether clang-format could actually break your code.
> This has actually always been the case especially since the introduction
> of include sorting, but also we all know that clang-format can break your
> code from the visual perspective too and hence the need for // clang-format
> off/on
>
> In the most part include sorting not only might break your code noisily
> such that it won't compile, but it can also break it silently,
> and because IncludeSorting is on by default this breakage could
> potentially go unnoticed.
>
> https://stackoverflow.com/questions/37927553/can-clang-format-break-my-code
> https://travisdowns.github.io/blog/2019/11/19/toupper.html
>
> I don't think it can be in any doubt that IncludeSorting is a nice feature
> used by 100,000's of developers without too many issues, but there is some
> suggestion that its inclusion as "on by default" was potentially a mistake.
>
> Proposals for other new features that modify the code in a similar way are
> getting some push back for changing the "charter" of clang-format when it
> could be considered to have already changed.
> This is causing friction on the review of some features and so it was
> suggested to present an RFC to gain wider consensus on the concept of
> clang-format changing code.
>
> Mostly when a code modifying change is submitted the view is that this
> isn't clang-formats job but more clang-tidy, however clang-tidy requires
> full access to all include files and compiler definitions and only works on
> the preprocessor paths of the code you are analyzing for and its speed and
> hence its frequency of use is drastically reduced.
>
> Some clang-format based modifications can in theory be made with a
> relatively high level of confidence without paying the price and the
> configuration complexity of getting all the
> semantic information. https://reviews.llvm.org/D105701.
> There is potentially for clang-format to introduce breaking changes and
> whilst this could in theory cause noisy breakages they could also in theory
> produce silent ones.
>
> These new features want to be run at "reformat" speeds & frequency and
> benefit from the rich Ecosystem of inclusion and integration in IDEs and
> editors that clang-format enjoys.
>
> This RFC is to try to gain some consensus as to what clang-format can do
> and what the conditions/constraints should be if allowed to do so.
>
> Benefits
> ========
>
> The benefits are that clang-format can be used to further make code
> modifications to adhere to a coding convention (insertion/removal of
> braces),
> clang-format could be used to validate and correct coding convention
> (left/right const),  and could be used to remove unnecessary semicolons or
> potentially convert code to trailing return types all of which could be
> performed at "reformat" speeds.
>
> Whilst some of these capabilities are available in clang-tidy, it requires
> significant infrastructure to often perform these often relatively simple
> operations and it's unlikely
> that all users of clang-format are set up to perform these actions in
> clang-tidy.
>
> There are likely a number of clang-tidy modifications that could in theory
> be made at "reformat" speeds with such an approach. But there really needs
> some agreement that it's OK for clang-format to modify the code.
>
> Allowing these kinds of modification capabilities could lead to a new set
> of "Resharper" style capabilities being added to clang-format,
> capable of bringing source code quickly into line with coding conventions.
>
> Concerns
> ========
>
> Correctness is King, the concern is your formatting tool should not
> perform operations that could break your code. (this is already the case)
>
> It's perhaps not clang-format's job to do this.
>
> I should personally declare myself as being in favor of allowing
> clang-format to modify code, I think it only fair that I let others reply
> to the RFC with their own concerns.
>
> Constraints
> ===========
>
> To minimize the impact to existing users, We suggest that a number of
> constraints be generally considered good practice when submitting reviews
> for clang-format with modifying changes
>
> 1) Code Modifying Features should always be off by default
> The user should have to make a positive affirmative action to use such a
> feature
>
> 2) Code Modifying Features configuration options should be highlighted as
> such in the ClangFormatStyleOptions.rst such that its clear these are
> potentially code breaking options
>
> 3) Existing "Code Modifying Features" do not have to adhere to 1) but the
> documentation should be updated to adhere to 2)
>
> 4) Code Modifying Features should be conservative to be "correct first"
> before being "complete".
> i.e. If it's possible a change could be ambiguous it should tend towards
> not making the incorrect change at all rather than forcing an incorrect
> change. (This could cause some
>     cases to be missed)
>
> Request
> =======
>
> I would like to get some general agreement that it's OK for future reviews
> of code modification changes to become part of clang-format (as they are in
> IncludeSorting) assuming the best practices are
> followed to protect users from unwanted changes.
>
> Feedback would be appreciated
>
> MyDeveloperDay
>
>
>
>
>
>
>
>
>
>
>
>
> _______________________________________________
> 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/20210810/65914f87/attachment-0001.html>