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

+some of the folks involved in the early development of clang-format who
might have more context on those design principles/choices made back then
(& perhaps on the include reordering change/tradeoff/choices made there)

Anyone got a link to the include reordering change/design discussion for
it? Might be informative to see if there was precedent/philosophy clearly
stated about how that was acceptable and what principles were applied &
could be reapplied to more recent proposals.

I tend towards the status quo (in general - that's just my personality) and
tend to prefer the fairly firm line clang-format currently has. But I don't
have a great rationale for "why include reordering but not const reordering
or dropping braces, etc" - marginally: Changing C++ syntax is more
complicated than reordering headers. Despite the fact that reordering
headers can have a big impact/breakage, it's hopefully fairly clear if/when
that happens that headers should be made order-agnostic. The subtleties of
what might happen when adding/removing braces if the heuristic
quick-parsing of clang-format fail seem likely to be more unfortunate, to
me at least. But maybe we're at a point where clang-format is widely
adopted/tested enough that that's less likely to have subtle mis-parse
breakages for terribly long? Not sure.

- Dave

On Mon, 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210809/1e3432dd/attachment-0001.html>

On Mon, Aug 9, 2021 at 3:55 PM David Blaikie via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> +some of the folks involved in the early development of clang-format who
might have more context on those design principles/choices made back then (&
perhaps on the include reordering change/tradeoff/choices made there)
>
> Anyone got a link to the include reordering change/design discussion for
it? Might be informative to see if there was precedent/philosophy clearly stated
about how that was acceptable and what principles were applied & could be
reapplied to more recent proposals.
Commit:
https://github.com/llvm/llvm-project/commit/d89ae9d3accb4cb8b7357932c3dfba8f4ae383c4

Patch discussion:
https://reviews.llvm.org/D11240

It doesn't look like there was any explicit discussion about the
potential for breaking code (but maybe I missed some, and maybe some
discussion happened in a different thread I've not yet found).

~Aaron
>
> I tend towards the status quo (in general - that's just my personality)
and tend to prefer the fairly firm line clang-format currently has. But I
don't have a great rationale for "why include reordering but not const
reordering or dropping braces, etc" - marginally: Changing C++ syntax is
more complicated than reordering headers. Despite the fact that reordering
headers can have a big impact/breakage, it's hopefully fairly clear if/when
that happens that headers should be made order-agnostic. The subtleties of what
might happen when adding/removing braces if the heuristic quick-parsing of
clang-format fail seem likely to be more unfortunate, to me at least. But maybe
we're at a point where clang-format is widely adopted/tested enough that
that's less likely to have subtle mis-parse breakages for terribly long? Not
sure.
>
> - Dave
>
> On Mon, 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