[PATCH] D127270: [clang-format] Add space in placement new expression

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jun 11 09:21:11 PDT 2022


MyDeveloperDay added inline comments.


================
Comment at: clang/include/clang/Format/Format.h:3498
+    /// \endcode
+    bool AfterPlacementNew;
     /// If ``true``, put a space between operator overloading and opening
----------------
HazardyKnusperkeks wrote:
> Please sort after `AfterOver...` here and all other occasions.
For new options they tend to go through a live cycle   bool->Enum->Structure...

can I as we start with an enum not a bool and do something like `Always,None,Leave` so Leave will just leave the code completely as it is today..

We get a lot of criticism for changing the defaults, often we are not changing them we are fixing things or now we format where we never did before.

I think it helps (as a concession to those who want us to NEVER change anything), if new options can have a `Leave` option so people don't get one or the other it just leaves the code neither formatted with a space or without a space, but doing what clang-format did before.

i.e. in the past we've been asked to be able to turn off/on individual styles. by ensuring we always have a "Leave" option we can guard the changes and in theory not introduce a change in people current formatting when using a later binary.


================
Comment at: clang/include/clang/Format/Format.h:3539
           AfterFunctionDefinitionName(false), AfterIfMacros(false),
-          AfterOverloadedOperator(false), AfterRequiresInClause(false),
-          AfterRequiresInExpression(false), BeforeNonEmptyParentheses(false) {}
+          AfterPlacementNew(true), AfterOverloadedOperator(false),
+          AfterRequiresInClause(false), AfterRequiresInExpression(false),
----------------
swap


================
Comment at: clang/include/clang/Format/Format.h:3550
              AfterIfMacros == Other.AfterIfMacros &&
+             AfterPlacementNew == Other.AfterPlacementNew &&
              AfterOverloadedOperator == Other.AfterOverloadedOperator &&
----------------
move down one P comes after O `AfterO -> AfterP`


================
Comment at: clang/lib/Format/Format.cpp:938
     IO.mapOptional("AfterIfMacros", Spacing.AfterIfMacros);
+    IO.mapOptional("AfterPlacementNew", Spacing.AfterPlacementNew);
     IO.mapOptional("AfterOverloadedOperator", Spacing.AfterOverloadedOperator);
----------------
move down one line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127270



More information about the cfe-commits mailing list