[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. 1. this needs a full context diff (diff -U 99) 2. this needs unit tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 ___ c

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 307804. anastasiia_lukianenko added a comment. 1. Diff was made by git diff with full context 2. Added description in documentation 3. Added unit test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D9

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. Do you think we should show what it looks like with more complex asm examples in the tests? what would the following look like? ` asm volatile("rbit %[aVal], %[aVal]" : [aVal] "+r"(val) : "[aVal]"(val));` `__asm__ __volatile__("str x0, %0" : "=m"(regs->regs[0]));

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko added a comment. Nothing will happen with your examples, because the line length is less than column limit. But if I make parameters longer results will be: BreakBeforeInlineASMColon = false; asm volatile("rbit %[aVal], %[aVal]" : [aVal] "+r"(val)

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. 1. You need to default initialize BreakBeforeInlineASMColon in the LLVM style. FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { ... LLVMStyle.BreakBeforeInlineASMColon = false; ... 2. You need a "CHECK_PARSE_BOOL" unit

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. My understanding is that the following current behavour ` asm("AA" : "DEF" : "GHI"); asm volatile( "AAA" : "DEF" : "

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko added a comment. For now without my patch current behavior is the following: Your examples listed below: asm("AA" : "DEF" : "GHI"); asm volatile( "AAA

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. Maybe I'm confused if you the setting is to break before ASM Colon then I expect it to always break, is that not what is required? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 _

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko added a comment. This option was created to have an option of removing this break, because without my patch it breaks before ASM colon in long lines. That's why I saved the behavior of BreakBeforeInlineASMColon=true as it was initially. Therefore, I'm asking you is this a b

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. I think my expectation would be that if you are going to have an option `BreakBeforeInlineASMColon` then it should do it for all case long and short, otherwise its not going to be clear. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://r

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 361134. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp clang/l

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:3800 + FormatStyle Style = getLLVMStyle(); + Style.BreakBeforeInlineASMColon = true; verifyFormat( I already gave my go in the past, but now I have to wonder, why are

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:3800 + FormatStyle Style = getLLVMStyle(); + Style.BreakBeforeInlineASMColon = true; verifyFormat( HazardyKnusperkeks wrote: > I already gave my go in the past, bu

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. Then I am sorry that I've missed that before. But you need to change your `bool` to an `enum` and also model the current behavior, so that there is no change

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 362680. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp clang/l

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added a comment. And format what clang-format says. Then it looks good to me. Comment at: clang/unittests/Format/FormatTest.cpp:6250 + verifyFormat("asm volatile(\"lng\",\n" + " : : val);", + Style)

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 363418. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp clang/l

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1022 LLVMStyle.BreakBeforeConceptDeclarations = true; + LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; LLVMStyle.BreakBeforeTernaryOperators = true; Are

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko added inline comments. Comment at: clang/lib/Format/Format.cpp:1022 LLVMStyle.BreakBeforeConceptDeclarations = true; + LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; LLVMStyle.BreakBeforeTernaryOperators = true; --

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added a comment. Then we can push it, do you need some one to do that? If yes please post name and email. Comment at: clang/unittests/Format/FormatTest.cpp:18475 +)", +Style); } I think the pre merge lint means this line. Please fix that an

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 334918. anastasiia_lukianenko edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/For

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 337433. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/Toke

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5077 + verifyFormat("asm volatile(\"string\", : : val);", Style); + verifyFormat("asm volatile(\"string\", : val1 : val2);", Style); + You should add a test case with lo

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 338958. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp clang/l

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko added a comment. @HazardyKnusperkeks @MyDeveloperDay Could you please review the last version? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 ___ cfe-commits mailing list cfe-com

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. Looks good, but please fix the clang-format notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

anastasiia_lukianenko updated this revision to Diff 339951. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp clang/l

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. LGTM but please wait for @MyDeveloperDay opinion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 ___ cfe-commits mailing

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added a comment. Herald added a project: All. If you still want this to be landed, we need a name and email for the commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 ___ cfe-co

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added a subscriber: owenpan. HazardyKnusperkeks added a comment. @MyDeveloperDay @owenpan What shall we do? 1. Abandon this 2. Commit it in our name 3. Use the email from this commit: https://github.com/Wandalen/game_chess/commit/a84bba03667643ded0cfe7aff2907c6546a6a192 I'm p

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

owenpan added a comment. In D91950#3915777 , @HazardyKnusperkeks wrote: > @MyDeveloperDay @owenpan What shall we do? > > 1. Abandon this > 2. Commit it in our name > 3. Use the email from this commit: > https://github.com/Wandalen/game_chess/commit/a84bb

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

MyDeveloperDay added a comment. I think this is them.. https://twitter.com/xen_org/status/1397581514318225412 https://xenproject.org/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 ___ cfe-commits m

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

HazardyKnusperkeks added a comment. In D91950#3917635 , @MyDeveloperDay wrote: > I think this is them.. > > https://twitter.com/xen_org/status/1397581514318225412 > https://xenproject.org/ > https://xenproject.org/2021/09/22/clang-format-for-xen-coding-st

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

This revision was automatically updated to reflect the committed changes. Closed by commit rGf6b252978c40: [clang-format] Add BreakBeforeInlineASMColon configuration (authored by anastasiia_lukianenko, committed by HazardyKnusperkeks). Changed prior to commit: https://reviews.llvm.org/D91950?v