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

[omar ahmed via Phabricator via cfe-commits]

omarahmed added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:1314
   LLVMStyle.SpaceBeforeParensOptions.AfterIfMacros = true;
+  LLVMStyle.SpaceBeforeParensOptions.AfterPlacementNew = true;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
----------------
HazardyKnusperkeks wrote:
> This isn't needed, because the default CTor initializes it with true.
> Or you change that, I don't know right now why all other attributes are initialized with false.
I think they are initialized with false so that when we come to [this case](https://github.com/llvm/llvm-project/blob/54ae4ca7550a81fd1fa9e484904d553af8fbb2fd/clang/lib/Format/Format.cpp#L1155), it breaks while all of them are false. (I am not sure so) I will try to add a test to cover 'SBPO_Never' but after converting it to enum.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3399
+      return Style.SpaceBeforeParensOptions.AfterPlacementNew ||
+             spaceRequiredBeforeParens(Right);
     if (Line.Type == LT_ObjCDecl)
----------------
MyDeveloperDay wrote:
> what case is covered here by spaceRequiredBeforeParens(Right)?
I intended to cover this case: `new (buf) T` so basically placement new expressions, excluding the overloaded new operator function declarations/definitions like `T *operator new(size_t size) {}` and excluding function declarations/definitions named new like `T *new();`


================
Comment at: clang/unittests/Format/FormatTest.cpp:15276
 
+  FormatStyle SpacePlacementNew = getLLVMStyle();
+  SpacePlacementNew.SpaceBeforeParens = FormatStyle::SBPO_Custom;
----------------
MyDeveloperDay wrote:
> curdeius wrote:
> > Are there any tests with `AfterPlacementNew = false;`? Could you add those please?
> you should probably give yourself your own test, for SpacePlacementNew
I was thinking of covering `placement delete` too and adding the tests for this patch to [this test](https://github.com/llvm/llvm-project/blob/54ae4ca7550a81fd1fa9e484904d553af8fbb2fd/clang/unittests/Format/FormatTest.cpp#L10153) as it should add more cases to new/delete operators. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127270