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

This revision was automatically updated to reflect the committed changes. Closed by commit rG7c15dd60ec45: [clang-format] Add space in placement new expression (authored by omarahmed, committed by owenpan). Changed prior to commit: https://reviews.llvm.org/D127270?vs=445172&id=557807#toc Repos

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

owenpan added inline comments. Herald added a reviewer: rymiel. Comment at: clang/include/clang/Format/Format.h:3555 AfterFunctionDefinitionName(false), AfterIfMacros(false), - AfterOverloadedOperator(false), AfterRequiresInClause(false), - AfterRequi

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

omarahmed updated this revision to Diff 445172. omarahmed added a comment. Change removes to remove Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 Files: clang/docs/ClangFormatStyleOptions.rst clang/

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

curdeius accepted this revision. curdeius added inline comments. Comment at: clang/include/clang/Format/Format.h:3504 +enum AfterPlacementOperatorStyle : int8_t { + /// Removes space after ``new/delete`` operators and before ``(``. + /// \code We ha

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

omarahmed added a comment. I don't have push credentials so If everything is okay with the patch, can you push it for me. My email is omarpiratee2...@gmail.com Comment at: clang/include/clang/Format/Format.h:3555 AfterFunctionDefinitionName(false), AfterIfMacros(fal

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

omarahmed updated this revision to Diff 438962. omarahmed added a comment. Format files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/tools/d

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

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3555 AfterFunctionDefinitionName(false), AfterIfMacros(false), - AfterOverloadedOperator(false), AfterRequiresInClause(false), - AfterRequiresInExpression(false)

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

omarahmed updated this revision to Diff 438719. omarahmed added a comment. Format files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/tools/d

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

omarahmed added inline comments. Comment at: clang/include/clang/Format/Format.h:3559 AfterFunctionDefinitionName(false), AfterIfMacros(false), - AfterOverloadedOperator(false), AfterRequiresInClause(false), - AfterRequiresInExpression(false), BeforeN

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

omarahmed updated this revision to Diff 438657. omarahmed added a comment. Change the default to APO_Leave Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 Files: clang/docs/ClangFormatStyleOptions.rst

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the Screenshot, please remember to mark your comments as "Done" once you've addressed them. From what I can tell this looks good. Comment at: clan

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

omarahmed marked 3 inline comments as done and an inline comment as not done. omarahmed added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3396 spaceRequiredBeforeParens(Right); +if (Style.SpaceBeforeParens == FormatStyle::SBPO_Custom && +

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

omarahmed added inline comments. Comment at: clang/docs/tools/dump_format_style.py:317 pass + elif state == State.InNestedEnum: +if line.startswith('///'): MyDeveloperDay wrote: > Can you show us a screenshot of how these changes will look

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

omarahmed updated this revision to Diff 438433. omarahmed added a comment. - Add version for nestedEnums and nestedFields - Make tests valid Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 Files: clang/

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

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:317 pass + elif state == State.InNestedEnum: +if line.startswith('///'): Can you show us a screenshot of how these changes will look in HTML? Reposit

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

HazardyKnusperkeks added a comment. Nice work adding the capability to the dump script. Comment at: clang/docs/ClangFormatStyleOptions.rst:4264 +and opening parentheses. +14 + This seems to be not entirely correct yet. :) Comment at:

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

omarahmed updated this revision to Diff 437980. omarahmed added a comment. Add Parse check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127270/new/ https://reviews.llvm.org/D127270 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/tool

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

omarahmed added a comment. As I understand, the default behavior for when the user didn't use `SBPO_Custom` is to add a space in placement operators based on this issue . And, at the same time, the default behavior should be `APO_Never` when we

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

omarahmed updated this revision to Diff 437966. omarahmed marked an inline comment as not done. omarahmed added a comment. Refactor the tests and add new parsing logic for nested enums in dump_format_style.py Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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

omarahmed added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:4261 + **AfterPlacementOperator** (``AfterPlacementOperatorStyle``) :versionbadge:`clang-format 14` +Defines in which cases to put a space between new/delete operators and opening parenth

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

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3495 +/// \code +///true: false: +///new (buf) T;vs.new(buf) T; MyDeveloperDay wrote: > should t

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

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:3495 +/// \code +///true: false: +///new (buf) T;vs.new(buf) T; should this be `Always/Never`

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

MyDeveloperDay added a comment. I think you are missing a PARSE unit test Comment at: clang/docs/ClangFormatStyleOptions.rst:4261 + **AfterPlacementOperator** (``AfterPlacementOperatorStyle``) :versionbadge:`clang-format 14` +Defines in which cases to put a space betwee

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

omarahmed added a comment. In D127270#3584200 , @curdeius wrote: > Does this patch really fix https://github.com/llvm/llvm-project/issues/54703? > If so, please add test for it. Otherwise remove the link from the summary > (and if possible handle it in a

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

omarahmed updated this revision to Diff 437468. omarahmed edited the summary of this revision. omarahmed added a comment. Add coverage for placement delete expressions and transform bool option to enum Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

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

curdeius added a comment. Does this patch really fix https://github.com/llvm/llvm-project/issues/54703? If so, please add test for it. Otherwise remove the link from the summary (and if possible handle it in another review). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:1314 LLVMStyle.SpaceBeforeParensOptions.AfterIfMacros = true; + LLVMStyle.SpaceBeforeParensOptions.AfterPlacementNew = true; LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true; --

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

omarahmed added inline comments. Comment at: clang/lib/Format/Format.cpp:1314 LLVMStyle.SpaceBeforeParensOptions.AfterIfMacros = true; + LLVMStyle.SpaceBeforeParensOptions.AfterPlacementNew = true; LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true; Hazard

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

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 `Aft

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

MyDeveloperDay added a comment. Thank you for the patch, was there a github issue for this? Can we just validate that those requirements are covered here too while we are at it? https://github.com/llvm/llvm-project/issues/54703 https://github.com/llvm/llvm-project/issues/41501 ( I know this is

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

HazardyKnusperkeks 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 Please sort after `AfterOver...` here and all

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

curdeius added a comment. Apart from some missing tests, looks promising! Comment at: clang/unittests/Format/FormatTest.cpp:15276-15286 + FormatStyle SpacePlacementNew = getLLVMStyle(); + SpacePlacementNew.SpaceBeforeParens = FormatStyle::SBPO_Custom; + SpacePlacementNew.Spa

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

omarahmed created this revision. omarahmed added a reviewer: MyDeveloperDay. Herald added a project: All. omarahmed requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add AfterPlacementNew option to SpaceBeforeParensOptions to have more contro