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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
