[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 22 12:00:20 PDT 2019


MyDeveloperDay added a comment.

@lebedev.ri Are we talking about a general ideology of the long term cost to allow any new things in? or to not allow things in this specific case?

because in this specific case all the changes are based on what is really a single clause that was already there before, namely

  Style.SpaceBeforeParens == FormatStyle::SBPO_Always

Now that clause is abstracted away to be (the abstraction in my view being a nice part of this change, becuase it avoided duplication and made it easier to reason about!)

  Style.SpaceBeforeParens == FormatStyle::SBPO_Always  ||
  (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses && Right.ParameterCount > 0);

i.e. always add a space, or add a space only when the number of parameters is > 0 and I don't want a space on empty parameters. (simple enough!)

So whilst I understand the arguments of code complexity, I feel it is sometimes used to discourage, what is in at least my view seemingly reasonable requests. (not my choice but reasonable all the same)

This was why I asked what was meant by "cost", but you raise the other cost, and that is one that is more legitimate... or is it?. have we seen it here as a problem or do we just fear it?

Having come from almost nothing to understanding a certain amount of of the code recently in a relatively short space of time (enough to at least contribute a few bug fixes), I'm confident that the "up to speed" costs are actually currently relatively low, and as the Format Library is only 20,000 lines and Format.cpp being only ~2500, I don't consider that to be large enough for the code to have come intractable already.

Of course I understand when someone starts asking for an Option e.g.

`AllowSpaceAfterSecondCommaOfArgumentListButNotBeforeAnyOfTheOthers: true`

We may have gone too far ;-), but I almost think that perhaps those cases might be more limited than we think in the end.

>From my recent experiences one nod to complexity was from the C++ language itself.. when trying to fix a bug in `isFunctionDeclarationName()` e,g,

  A foo(abc);

does the mean a function foo() taking an argument abc and returning A or an instance of class A called foo being constructed with an argument abc.. This can have all the difference if you want to put a `BraceAfterReturnType`

The "cost" as you describe it here, over our ability to reason about the code in my view is more likely to come from having to analyze snippets of languages without the full parser to reason with.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55170/new/

https://reviews.llvm.org/D55170





More information about the cfe-commits mailing list