[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MarcusJohnson91 added a comment. Not trying to take over this patch, because I've got a ton of patches I need to be finishing up myself. but I think the problematic code is in UnwrappedLineParser.cpp:1900 if (FormatTok->Tok.getKind() == ClosingBraceKind) { if (IsEnum &&

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MarcusJohnson91 added a comment. In D93938#2832825 , @Userbla wrote: > I applied this fix locally to a branch based off llvm 11.x and the > `FormatTest.FormatsTypedefEnum` test now fails. I'm running into this bug too. typedef enum Blah { One =

[PATCH] D106756: Added l16/l32 length modifiers for char16_t/char32_t

MarcusJohnson91 updated this revision to Diff 387788. MarcusJohnson91 added a comment. Clang-FormatDiff.py; Still waiting for the UTFConvert patch to land first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106756/new/ https://reviews.llvm.org/D106756 Files:

[PATCH] D106755: Extended format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 363376. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106755/new/ https://reviews.llvm.org/D106755 Files: clang-tools-extra/clang-tidy/boost/UseToStringCheck.cpp clang-tools-extra/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp

[PATCH] D106756: Added l16/l32 length modifiers for char16_t/char32_t

MarcusJohnson91 updated this revision to Diff 363374. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106756/new/ https://reviews.llvm.org/D106756 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/FormatString.h clang/lib/AST/FormatString.cpp

[PATCH] D106755: Extended format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 363245. MarcusJohnson91 added a comment. Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106755/new/ https://reviews.llvm.org/D106755 Files: clang-tools-extra/clang-tidy/boost/UseToStringCheck.cpp

[PATCH] D106756: Added l16/l32 length modifiers for char16_t/char32_t

MarcusJohnson91 updated this revision to Diff 363244. MarcusJohnson91 added a comment. Rebased on Main CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106756/new/ https://reviews.llvm.org/D106756 Files: clang/include/clang/AST/FormatString.h clang/lib/AST/FormatString.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. There is only one function in ConvertUTFWrapper.cpp: convertUTF32ToUTF8String idk wtf is going on, maybe the ammending the commit is breaking something? the diff I see here is correct... Maybe I should just make a new diff here entirely? CHANGES SINCE LAST

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 363216. MarcusJohnson91 added a comment. It seems like this diff keeps getting reverted? I've fixed all the issues mentioned, and the tests work now, everything is formatted correctly too. I've set git up to do full context diffs, but it's not

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362923. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp llvm/unittests/Support/ConvertUTFTest.cpp Index:

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362907. MarcusJohnson91 added a comment. Formatted the diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362882. MarcusJohnson91 added a comment. The tests work on my machine now, turns out the Big endian one needs a BOM, pretty obvious in hindsight. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. The problem seems to be in the conversion function expecting strings to be a multiple of 4 bytes, which doesn't hold up with the way ArrayRef stores things as char that is casted to char32_t, when using ASCII values like in the look of disapproval emoji, having

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362511. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp llvm/unittests/Support/ConvertUTFTest.cpp Index:

[PATCH] D106755: Extended format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 362446. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106755/new/ https://reviews.llvm.org/D106755 Files: clang-tools-extra/clang-tidy/boost/UseToStringCheck.cpp clang-tools-extra/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp

[PATCH] D106756: Added l16/l32 length modifiers for char16_t/char32_t

MarcusJohnson91 updated this revision to Diff 362444. MarcusJohnson91 added a comment. Added a couple tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106756/new/ https://reviews.llvm.org/D106756 Files: clang/include/clang/AST/FormatString.h clang/lib/AST/FormatString.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362431. MarcusJohnson91 added a comment. Updated the tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added inline comments. Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:176 + // enough that we can fit a null terminator without reallocating. + Out.resize(SrcBytes.size() * UNI_MAX_UTF8_BYTES_PER_CODE_POINT + 1); + UTF8 *Dst = reinterpret_cast([0]);

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362198. MarcusJohnson91 added a comment. Dropped the UTF32 BOM stuff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. Anyone got any ideas what happened this time? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 marked 5 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/lib/AST/Expr.cpp:1091 + if (llvm::convertUTF32ToUTF8String(AR, Output)) { +CString = new char[Output.size() + 1]; +memcpy(CString, Output.c_str(),

[PATCH] D106756: Added l16/l32 length modifiers for char16_t/char32_t

MarcusJohnson91 updated this revision to Diff 361539. MarcusJohnson91 added a comment. Clang-formatted the diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106756/new/ https://reviews.llvm.org/D106756 Files: clang/include/clang/AST/FormatString.h clang/lib/AST/FormatString.cpp

[PATCH] D106755: Extended format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361538. MarcusJohnson91 added a comment. Clang-formatted the diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106755/new/ https://reviews.llvm.org/D106755 Files: clang-tools-extra/clang-tidy/boost/UseToStringCheck.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 361535. MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Implemented the fixes mentioned and reformatted the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753

[PATCH] D106755: Extended format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361534. MarcusJohnson91 added a comment. Herald added subscribers: llvm-commits, dexonsmith, hiraditya. Herald added a project: LLVM. Implemented the fixes mentioned and reformatted the patch CHANGES SINCE LAST ACTION

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 marked 3 inline comments as done. MarcusJohnson91 added inline comments. Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:168 + std::vector ByteSwapped; + if (Src[0] == UNI_UTF16_BYTE_ORDER_MARK_SWAPPED) { +ByteSwapped.insert(ByteSwapped.end(), Src,

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. I don't understand why the build failed? I've compiled it and ran all the tests with `time ninja check` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 ___

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 361487. MarcusJohnson91 edited the summary of this revision. MarcusJohnson91 added a comment. Added tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files:

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 361485. MarcusJohnson91 added a comment. Added tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp

[PATCH] D106756: Added l16/l32 length modifiers for char16_t/char32_t

MarcusJohnson91 created this revision. MarcusJohnson91 added reviewers: aaron.ballman, efriedma. MarcusJohnson91 added a project: clang. MarcusJohnson91 requested review of this revision. Split from D103426 Repository: rG LLVM Github Monorepo

[PATCH] D106755: Extended format string checking to wprintf/wscanf

MarcusJohnson91 created this revision. Herald added a subscriber: martong. Herald added a reviewer: aaron.ballman. MarcusJohnson91 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Split from D103426

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 created this revision. MarcusJohnson91 added a project: clang. Herald added subscribers: dexonsmith, hiraditya. MarcusJohnson91 requested review of this revision. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Split from D103426

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361466. MarcusJohnson91 added a comment. Full context diff after squashing all the commits together CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/ https://reviews.llvm.org/D103426 Files:

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361414. MarcusJohnson91 added a comment. Getting weird crashes all over the place in code I didn't touch, no idea what's going on CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/ https://reviews.llvm.org/D103426 Files:

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361410. MarcusJohnson91 added a comment. Rebased on main CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/ https://reviews.llvm.org/D103426 Files: clang/lib/AST/OSLog.cpp clang/lib/Sema/SemaChecking.cpp

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added inline comments. Comment at: clang/lib/AST/TemplateBase.cpp:77-80 + const Decl *D = nullptr; + if (T->isTypedefNameType()) { +D = T->getAs()->getDecl(); + } aaron.ballman wrote: > This approach seems fragile for getting access to an

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361409. MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. #1: Moved a comment to the top, so it's brettier  #2: Moved all the ugly StringLiteral conversion code to StringLiteral::getStrDataAsChar and forwarded that

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 361405. MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Just implemented the change Aaron requested in TemplateBase.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 359704. MarcusJohnson91 added a comment. Herald added subscribers: llvm-commits, dexonsmith, hiraditya. Herald added a project: LLVM. Few tweaks since last time, nothing big CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 updated this revision to Diff 359590. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/ https://reviews.llvm.org/D103426 Files: clang/include/clang/AST/Expr.h clang/include/clang/AST/FormatString.h clang/include/clang/AST/Type.h clang/lib/AST/Expr.cpp

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added inline comments. Comment at: clang/include/clang/AST/Expr.h:1846-1871 + std::string getStringAsChar() const { +assert(getCharByteWidth() == 1 && + "This function is used in places that assume strings use char"); +return

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added inline comments. Comment at: clang/lib/AST/OSLog.cpp:212 + } else if (Lit->isUTF16()) { +std::wstring_convert, char16_t> Convert; +std::u16string U16 = Lit->getStringAsChar16(); aaron.ballman wrote: > cor3ntin wrote: > > I'm not

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/ https://reviews.llvm.org/D103426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added a comment. In D103426#2807860 , @aaron.ballman wrote: > In D103426#2807245 , > @MarcusJohnson91 wrote: > >> In D103426#2806391 , >>

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added inline comments. Comment at: clang/lib/AST/Type.cpp:1980-2002 +bool Type::isChar16Type(const LangOptions ) const { if (const auto *BT = dyn_cast(CanonicalType)) -return BT->getKind() == BuiltinType::Char16; - return false; -} - -bool

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 added a comment. In D103426#2806391 , @aaron.ballman wrote: > Do you have a reference to the WG14 paper proposing these conversion > specifiers? I've previously written up the proposal, which you actually helped with (thank you for

[PATCH] D103426: Clang: Extend format string checking to wprintf/wscanf

MarcusJohnson91 created this revision. MarcusJohnson91 created this object with visibility "All Users". MarcusJohnson91 added a project: clang. Herald added a subscriber: martong. Herald added a reviewer: aaron.ballman. MarcusJohnson91 requested review of this revision. Herald added a project:

[PATCH] D88084: [clang-format] Changed default styles BraceWrappping bool table to directly use variables

MarcusJohnson91 added a comment. @MyDeveloperDay This patch was merged upstream a long time ago, how do I close it here on Phabricator? thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88084/new/ https://reviews.llvm.org/D88084

[PATCH] D88084: [clang-format] Changed default styles BraceWrappping bool table to directly use variables

MarcusJohnson91 added a comment. In D88084#2287450 , @MyDeveloperDay wrote: > I noticed the pre-merge tests failed! Yeah I just noticed that too, not sure what's up but I'll check into it, and yeah that's a good idea about initializing some of these

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked an inline comment as not done. MarcusJohnson91 added inline comments. Comment at: clang/lib/Format/Format.cpp:586 IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);

[PATCH] D88084: [clang-format] Changed default styles BraceWrappping bool table to directly use variables

MarcusJohnson91 created this revision. MarcusJohnson91 added reviewers: MyDeveloperDay, sylvestre.ledru. MarcusJohnson91 added a project: clang-format. Herald added a project: clang. MarcusJohnson91 requested review of this revision. Which should make these defaults more immune to changes in the

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. @sylvestre.ledru After looking more closely at the issue, it seems you're having an issue with Mozilla's comment alignment option. you want the comments to be aligned, and it appears Clang11 no longer has that option set for Mozilla's style is what you're

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 293240. MarcusJohnson91 edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/lib/Format/Format.cpp Index: clang/lib/Format/Format.cpp

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2283961 , @sylvestre.ledru wrote: > Any chance this changes could have caused this regression > https://bugs.llvm.org/show_bug.cgi?id=47589 ? I don't think so, but I can double check the style defaults for the

[PATCH] D80214: [clang-format] Set of unit test to begin to validate that we don't change defaults

MarcusJohnson91 accepted this revision. MarcusJohnson91 added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80214/new/ https://reviews.llvm.org/D80214

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2044492 , @MyDeveloperDay wrote: > If you want me to land this for you, I'd feel more comfortable landing it if: > > a) We can land D80214: [clang-format] Set of unit test to begin to validate > that we don't

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264916. MarcusJohnson91 added a comment. Format.h: indented the ``AfterExternBlock: true`` example code snippet with 4 spaces like the Indent option so it's more visible and matches. I think it's perfect now. CHANGES SINCE LAST ACTION

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264909. MarcusJohnson91 added a comment. Just fixed the formatting of the ReleaseNotes.rst file, the extern blocks were slightly askew, and it might've made it a bit confusing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264911. MarcusJohnson91 added a comment. Made the IndentExternBlockStyle enum comments a bit clearer, and regenerated the .rst file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files:

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 5 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/lib/Format/Format.cpp:812 true, true}; + LLVMStyle.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2043758 , @MyDeveloperDay wrote: > Sorry to "go around the houses" but we'll get there in the end...I think we > are close I think we're close too. Your other comment was interesting, about testing the

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264839. MarcusJohnson91 added a comment. Ok, I've removed the inherited ones, and also removed the times I was setting a style when there wasn't one before. also I moved the `IEBS_AfterExternBlock` line to right underneath the

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 4 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/lib/Format/Format.cpp:714 case FormatStyle::BS_Mozilla: +Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; Expanded.BraceWrapping.AfterClass =

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. As for crashes, none of them seem relevant; I'm on MacOS, the windows ABI crash seems especially irrelevent. opt crashed, there were no arguments, and abort() was called. llvm-lto2 crashed, not-prevailing.ll.tmp1-3.bc was the cause llvm-dwarfdump crashed,

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264763. MarcusJohnson91 added a comment. Added the style initializers, moved IEBS_AfterExternBlock to be the first enum value so that it's zero, that way the bool logic works. Regenerated the docs as well, and also clang-formatting the files I've

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. I've initialized all styles to either AfterExternBlock, if there was a BraceWrapping block, or NoIndent if there wasn't. re-running my tests locally. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. I've got the indenting to work manually now as well, the issue was you need to have `BreakBeforeBraces: Custom` in the inline style for it to pick up BraceWrapping.AfterExternBlock's value. Now I'm working on the automated tests, thanks for the tip about

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264597. MarcusJohnson91 added a comment. Fixed the generation of ReleaseNotes.rst CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2040665 , @MyDeveloperDay wrote: > > Something is not quite right here, this text isn't ending up in the > ClangFormatStyleOptions.rst You're right, I didn't catch that before, turns out having a comment

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2040532 , @MyDeveloperDay wrote: > LGTM So what's the next step? I've never committed to LLVM before. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Removed the lowercase Noindent case, that was a last minute addition I thought might make it a tad easier to work with, but you're right I didn't even test it, and honestly adding that complexity is just

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264462. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264439. MarcusJohnson91 added a comment. Removed forgotten comment from control logic of UnwrappedLineParser CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files:

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264438. MarcusJohnson91 edited the summary of this revision. MarcusJohnson91 added a comment. Did everything you asked and did a littl bit of my own cleanup as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 3 inline comments as done. MarcusJohnson91 added a comment. I've fixed all of your comments as well as fixed the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 258612. MarcusJohnson91 edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. @MyDeveloperDay > but I'm also constantly surprised by how many of the enumeration cases > started out as booleans only later to have to be converted to enums. The more > I think about this the more I think the problem can probably be dealt with > better by

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. I agree that changing formatting randomly isn't a good idea, and I think converting AfterExternBlock to an enum is the way to go, but I'm just not sure on how it should be implemented. Ok, I've got an idea to deprecate the AfterExternBlock option and map it to

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MarcusJohnson91 added a comment. In D33029#1946761 , @bbassi wrote: > I don't think that's quite right. Then you will also have to have a > `AlignWithDanglingParenthesis` for cases when people still want closing > parenthesis on new line but want

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 252730. MarcusJohnson91 added a comment. Implemented the suggestion to break the test strings down into smaller pieces CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files:

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 2 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2497 Style); } MyDeveloperDay wrote: > my assumption is this test is using `Style.IndentExternBlock

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 252466. MarcusJohnson91 marked 4 inline comments as done. MarcusJohnson91 added a comment. Implemented @MyDeveloperDay's suggestion to simplify the if/else statements. Removed all the test changes except one: That's because the

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Restored all the test function names to foo(). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:31 +class FormatTest +: public ::testing::Test { // FormatTest is a Fixture, data is reused protected: MyDeveloperDay wrote: > is this comment necessary? Removed the

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. Rebased on Master again, recompiling and re-running all the tests. I'll update this comment when it passes, or create a new diff if it doesn't but nothing had to be changed so it'll probably work. @krasimir I noticed that you've been active recently, can you

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 251846. MarcusJohnson91 added a comment. Rebased on master CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. Bump CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 250354. MarcusJohnson91 added a comment. Fixed Format.h comments, and rebased on master. it's perfect on my end now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files:

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 249954. MarcusJohnson91 added a comment. Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 249838. MarcusJohnson91 added a comment. New squashed commit with all changes present CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#1917837 , @MyDeveloperDay wrote: > your patch seems to be missing the other files Which other files? I made a new commit and did the full context diff, now sure why it's only showing the documentation update.

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 248975. MarcusJohnson91 added a comment. Updated the release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst Index: clang/docs/ReleaseNotes.rst

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 248974. MarcusJohnson91 added a comment. Removed the debugging comments I added to FormatTest.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/include/clang/Format/Format.h

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#1911133 , @MyDeveloperDay wrote: > you need documentation and release note changes too The comments were only for testing, I'll remove them. The tests had to change because the behavior has changed slightly.

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 created this revision. MarcusJohnson91 added a reviewer: cfe-commits. Herald added a project: clang. and refactored the BraceWrapping.AfterExternBlock code so that it only deals with wrapping the brace after an extern block. Repository: rG LLVM Github Monorepo

[PATCH] D75410: [clang-format] Fixed BraceWrapping.AfterExternBlock

MarcusJohnson91 abandoned this revision. MarcusJohnson91 added a comment. I'm moving the intended change to a new clang-format option instead of modifying an established one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75410/new/ https://reviews.llvm.org/D75410

[PATCH] D75410: [clang-format] Fixed BraceWrapping.AfterExternBlock

MarcusJohnson91 updated this revision to Diff 247524. MarcusJohnson91 added a comment. Full Context Diff (I think?) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75410/new/ https://reviews.llvm.org/D75410 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D75410: [Clang-Format] Fixed BraceWrapping.AfterExternBlock

MarcusJohnson91 updated this revision to Diff 247462. MarcusJohnson91 added a comment. Rewrote the AfterExternBlock code to rely on just that clang-format option and remove the ExternNamespace check. Fixed the tests as well. and clang-formatted everything CHANGES SINCE LAST ACTION

[PATCH] D75410: Fixed Extern Block Indentation in libFormat

MarcusJohnson91 updated this revision to Diff 247457. MarcusJohnson91 added a comment. Clang-Formatted UnwrappedLineParser.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75410/new/ https://reviews.llvm.org/D75410 Files: clang/lib/Format/UnwrappedLineParser.cpp Index:

[PATCH] D75410: Fixed Extern Block Indentation in libFormat

MarcusJohnson91 added a comment. Not sure why it failed to compile, it compiles just fine on my machine... I'm gonna re-pull from master and see if it works. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75410/new/ https://reviews.llvm.org/D75410

  1   2   >