[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

owenpan added a comment. Something like the following: diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index 87515372046d..3dc5e411df55 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -98,6 +98,8 @@ namespace format {

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

owenpan added a comment. In D138263#3937223 , @HazardyKnusperkeks wrote: > In D138263#3936536 , > @HazardyKnusperkeks wrote: > >> In D138263#3936007 , @owenpan >> wrote

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

owenpan added a comment. I suppose it's fairly easy to annotate the `l_brace` of a namespace? If so, then wouldn't it be better to do that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138263/new/ https://reviews.llvm.org/D138263 ___

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3935856 , @goldstein.w.n wrote: > In D137181#3935752 , @owenpan wrote: > >> Please mark review comments as done if you have addressed them. Can you also >> clean up the test c

[PATCH] D138234: [clang-format] Support new file formatting with vim

owenpan added inline comments. Comment at: clang/tools/clang-format/clang-format.py:48 import vim +import os.path Can you move it up to keep the module names sorted? Comment at: clang/tools/clang-format/clang-format.py:80 lines = ['-lin

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838 + // If this changes PPLevel needs to be used for get correct indentation. + assert(!(Line.InMacroBody && InPPDirective)); return Line.Level * Style.IndentWidth + Length <= ColumnLimit; -

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. Please mark review comments as done if you have addressed them. Can you also clean up the test cases, removing overlapping/redundant ones, making sure a test case doesn't end with a newline (e.g., line 5380), etc? Comment at: clang/lib/Format/Unwrappe

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

owenpan accepted this revision. owenpan added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137762/new/ https://reviews.llvm.org/D137762 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

owenpan added a comment. @gedare thanks for changing `EXPECT_EQ` to `verifyFormat`, but IMO we should do that in another patch so that it would be easier (at least for me) to review the new tests and to make sure no existing tests have been changed by accident. Comment at: cl

[PATCH] D137823: [clang-format][NFC] Moved configuration parsing tests in own file

owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137823/new/ https://reviews.llvm.org/D137823 ___ cfe-commits mailing list cfe-comm

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3924002 , @sstwcw wrote: > Can you make `TokenAnnotator::printDebugInfo` print `PPLevel`? @goldstein.w.n can you add it as follows? $ git diff TokenAnnotator.cpp diff --git a/clang/lib/Format/TokenAnnotator.cpp b

[PATCH] D137823: [clang-format][NFC] Moved configuration parsing tests in own file

owenpan added inline comments. Comment at: clang/unittests/Format/ConfigParseTest.cpp:1006 + +TEST(FormatStyle, GetStyleWithEmptyFileName) { + llvm::vfs::InMemoryFileSystem FS; HazardyKnusperkeks wrote: > owenpan wrote: > > Otherwise, the test will be skipped. >

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. I think we are close to the finishing line. Can you revisit the change to the formatter and clean it up? For example, casting `PPLevel` to `unsigned` is redundant now. IMO you can simply the change too. Comment at: clang/lib/Format/UnwrappedLineFormat

[PATCH] D137865: [clang-format][NFC] Improve documentation on ReflowComments

owenpan added inline comments. Comment at: clang/include/clang/Format/Format.h:3864 /// \endcode + /// This option has only effect if ``ReflowComments`` is set to ``true``. /// \version 13 owenpan wrote: > We need an empty line after `/// \endcode`. > We n

[PATCH] D137865: [clang-format][NFC] Improve documentation on ReflowComments

owenpan added inline comments. Comment at: clang/include/clang/Format/Format.h:3073-3076 + /// If ``true``, clang-format will attempt to re-flow comments. That is it + /// will touch a comment and *reflow* long comments into new lines, trying to + /// obey the ``ColumnLimit``.

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3918715 , @goldstein.w.n wrote: > In D137181#3918673 , @owenpan wrote: > >> Below is how I defined `PPLevel`: >> >> $ git diff UnwrappedLineParser.h >> diff --git a/clang/l

[PATCH] D137823: [clang-format][NFC] Moved configuration parsing tests in own file

owenpan added inline comments. Comment at: clang/unittests/Format/ConfigParseTest.cpp:14 + +#define DEBUG_TYPE "parse-test" + You can delete it. Comment at: clang/unittests/Format/ConfigParseTest.cpp:1006 + +TEST(FormatStyle, GetStyleWithEmptyF

[PATCH] D137883: [clang-format][NFC] Improve documentation of FixNamespaceComments

owenpan added inline comments. Comment at: clang/include/clang/Format/Format.h:2124-2126 + /// namespaces and fixes invalid existing ones. This is omitted for short + /// namespaces, what a short namespace is, is controlled by + /// "ShortNamespaceLines". Or s

[PATCH] D137755: [clang-format] Treats &/&& as reference when followed by ',' or ')'.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe864ac694540: [clang-format] Treats &/&& as reference when followed by ',' or ')' (authored by red1bluelost, committed by owenpan). Repository: r

[PATCH] D137486: [clang-format] Correctly annotate function names before attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG96e23906b5e8: [clang-format] Correctly annotate function names before attributes (authored by owenpan). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3918117 , @goldstein.w.n wrote: >>> maybe you did something different? >> >> Here is what I did: >> >> $ git diff UnwrappedLineParser.cpp >> diff --git a/clang/lib/Format/UnwrappedLineParser.cpp >> b/clang/lib/For

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3916558 , @goldstein.w.n wrote: > In D137181#3916547 , @owenpan wrote: > >> Yes, if there is a header guard. Otherwise, set `PPLevel` to `PPBranchLevel >> + 1`. > > That fails

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.h:66 + /// #endifPPLevel still at : 0 + int PPLevel; + goldstein.w.n wrote: > owenpan wrote: > > You need to initialize `PPLevel` in the constructor. FWIW I'd use >

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:112 : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken), -PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource), -Token(nullptr), PreviousToken(nullptr)

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3916521 , @goldstein.w.n wrote: > I think we can remove the places I set `InitialPPLevel` and `OldPPLevel` but > anything else I remove causes at least one test to fail. > > What did you do? Just set `PPLevel = PPBran

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3916488 , @goldstein.w.n wrote: > In D137181#3916474 , @owenpan wrote: > >> You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, >> instead of adjusting

[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] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, instead of adjusting `PPLevel` at various places, you only need to set it once when `InMacroBody` is set. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:92-94 + if (

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3914100 , @goldstein.w.n wrote: > Err I'm not sure that's right. I thought we where going for the C-code should > start at the column that the 'd' in define is. > So it would be: > > #define X \ >switch (x

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3914083 , @goldstein.w.n wrote: > In D137181#3914066 , @owenpan wrote: > >> @goldstein.w.n do you need to modify MacroCallReconstructor.cpp, Macros.h, >> and MacroCallReconstr

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. @goldstein.w.n do you need to modify MacroCallReconstructor.cpp, Macros.h, and MacroCallReconstructorTest.cpp? Leaving them out wouldn't break any existing tests. Adding `PPBranchLevel` (or `PPLevel` in your case) to `UnwrappedLine` and `AnnotatedLine` worked for me. I

[PATCH] D137486: [clang-format] Correctly annotate function names before attributes

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:76 +static bool isCppAttribute(bool IsCpp, const FormatToken &Tok) { + if (!IsCpp || !Tok.startsSequence(tok::l_square, tok::l_square)) +return false; HazardyKnusperkeks wrote: >

[PATCH] D137409: [clang-format][NFCish] Alphabetical sort Format.(h|pp)

owenpan added a comment. Don't forget to run dump_format_style.py before landing. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137409/new/ https://reviews.llvm.org/D137409 ___ cfe-commits mailing lis

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3911005 , @sstwcw wrote: > Here is one problem: > > clang-format -style='{IndentPPDirectives: BeforeHash, PPIndentWidth: 1, > IndentWidth: 4, IndentCaseLabels: true}' > > actual: > #define X

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3910800 , @goldstein.w.n wrote: > In D137181#3910799 , @owenpan wrote: > >> IMO we should find a simpler way to indent bodies of macro definitions that >> are nested more than

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. IMO we should find a simpler way to indent bodies of macro definitions that are nested more than one level deep. I prefer we handle that in another patch and only support the examples in the summary for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D137474: [clang-format] Defer formatting of operator< to honor paren spacing

owenpan accepted this revision. owenpan added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/FormatTest.cpp:15476 + verifyFormat("bool operator< ();", Space); + verifyFormat("bool operator> ();", Space); verifyFormat("voi

[PATCH] D137486: [clang-format] Correctly annotate function names before attributes

owenpan added a comment. In D137486#3909970 , @rymiel wrote: > Are both isCpp11AttributeSpecifier and isCppAttribute required? Not really, but I've kept `isCpp11AttributeSpecifier` for now as it might become handy if it needs to be called at other place

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. In D137181#3910404 , @goldstein.w.n wrote: > Doesn't that add an arbitrary +1 to the begining of the indentation? > Shouldn't it be: > > // IndentPPDirectives: AfterHash > #ifdef foo > # define bar() \\ > if (A) { \

[PATCH] D137486: [clang-format] Correctly annotate function names before attributes

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, rymiel. owenpan added a project: clang-format. Herald added a project: All. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://gi

[PATCH] D137223: [clang-format] Remove special case for kw_operator when aligning decls

owenpan accepted this revision. owenpan 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/D137223/new/ https://reviews.llvm.org/D137223 _

[PATCH] D137308: [clang-format][NFC] Remove parsePPElIf()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe787708bcf53: [clang-format][NFC] Remove parsePPElIf() (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" goldstein.w.n wrote

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. With `ColumnLimit: 16` and `IndentPPDirectives: BeforeHash`, the format should be: #ifdef foo #define bar() \ if (A) { \ B(); \ } \ C(); #endif With `IndentPPDirectives: AfterHash`, it should look like: #ifdef foo #

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" I just noticed that

[PATCH] D137308: [clang-format][NFC] Remove parsePPElIf()

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, rymiel. owenpan added a project: clang-format. Herald added a project: All. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG117d792f35e6: [clang-format] Don't skip #else/#elif of #if 0 (authored by owenpan). Changed prior to commit: https://reviews.llvm.org/D137052?vs=4

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5047 + style.IndentPPDirectives = FormatStyle::PPDIS_None; + verifyFormat("#ifdef foo\n" + "#define bar() \\\n" goldstein.w.n wrote: > owenpan wrote: > > Do you need

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

owenpan added a comment. Can you fix the format of the summary? Comment at: clang/unittests/Format/FormatTest.cpp:5024 style); + verifyFormat("#if 1\n" Don't add an empty line here. Comment at: clang/unittests/Format/Format

[PATCH] D137223: [clang-format] Remove special case for kw_operator when aligning decls

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17939 Alignment); + // Ensure operators are not aligned if they're being called, not declared + verifyFormat("int main() {\n" Add a full stop. Co

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

owenpan added a comment. In D137052#3899091 , @aaron.ballman wrote: > Can you also add a test for `#elifdef` and `#elifndef`? I added a test (lines 6130-6136) for `#elif` but not `#elifdef` and `#elifndef` because all three are on the same execution pa

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

owenpan added a comment. In D137052#3895426 , @sstwcw wrote: > This patch fixes the regression caused by 2183fe2 > while > introducing a new regression. But in my opinion the new reg

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

owenpan updated this revision to Diff 472204. owenpan added a comment. Tweaked the fix and added a test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137052/new/ https://reviews.llvm.org/D137052 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp

[PATCH] D133647: [clang-format] Parse the else part of `#if 0`

owenpan added a comment. See D137052 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133647/new/ https://reviews.llvm.org/D133647 ___ cfe-commits mailing list cfe-commit

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

owenpan created this revision. owenpan added reviewers: sstwcw, sammccall, HazardyKnusperkeks, MyDeveloperDay, rymiel. owenpan added a project: clang-format. Herald added a project: All. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commit

[PATCH] D136830: [clang-format][NFC] Move BracesRemover tests out of FormatTest.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG30ea3fcc4c69: [clang-format][NFC] Move BracesRemover tests out of FormatTest.cpp (authored by owenpan). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D136154: [clang-format] Fix the continuation indenter

owenpan added a comment. In D136154#3882163 , @hel-ableton wrote: > I hope we can somewhat start from square 1 again with this. See https://reviews.llvm.org/D136154#3890747. It doesn't "fix" the last example in https://github.com/llvm/llvm-project/issu

[PATCH] D136154: [clang-format] Fix the continuation indenter

owenpan added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:803-804 CurrentState.LastSpace = State.Column; - } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr, - TT_CtorInitializerColon)) &&

[PATCH] D136830: [clang-format][NFC] Move BracesRemover tests out of FormatTest.cpp

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, rymiel. owenpan added a project: clang-format. Herald added a project: All. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D136658: [clang-format] Move InsertBraces unit tests out of FormatTest.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG17059753f133: [clang-format] Move InsertBraces unit tests out of FormatTest.cpp (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D136154: [clang-format] Fix the continuation indenter

owenpan added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:803-808 - } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr, - TT_CtorInitializerColon)) && +} else if (Previous.is(TT_CtorInitializerColon)) {

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1345 while (CurrentToken) { -if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator)) +if (IsMarkOrRegion || CurrentToken->Previous->is(TT_BinaryOperator)) { Curre

[PATCH] D136658: [clang-format] Move InsertBraces unit tests out of FormatTest.cpp

owenpan added a comment. In D136658#3883372 , @rymiel wrote: > Is the intent here to somewhat reduce the 30k line behemoth of FormatTest.cpp? See https://reviews.llvm.org/D136437#3877250 for the motivation. In D136658#3883511

[PATCH] D135972: [clang-format] Don't crash on malformed preprocessor conditions

owenpan accepted this revision. owenpan added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/FormatTest.cpp:5209 + verifyNoCrash("#if X\n" +"#else\n" +"#else\n" `#elif Y` inste

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

owenpan accepted this revision. owenpan 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/D136336/new/ https://reviews.llvm.org/D136336 _

[PATCH] D136337: [clang-format] Discard pre-processor statements in parseBracedList()

owenpan added a comment. Can you create an issue on GitHub and include the details on how to reproduce the problem using the latest clang-format? Comment at: clang/unittests/Format/FormatTest.cpp:19975-19978 + verifyFormat("CxxClass::CxxClass() {\n#pragma region test(hello)\n

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

owenpan added a comment. Can you run git-clang-format? Comment at: clang/lib/Format/TokenAnnotator.cpp:1340 Keywords.kw_region)) { - bool IsMark = CurrentToken->is(Keywords.kw_mark); + bool IsMarkOrRegion = CurrentToken->isOneOf(Keywords

[PATCH] D135972: [clang-format] Don't crash on malformed preprocessor conditions

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1217 + assert(PPBranchLevel >= -1); + if (PPBranchLevel <= -1) +conditionalCompilationStart(/*Unreachable=*/true); HazardyKnusperkeks wrote: > You assert >= -1, that means t

[PATCH] D136635: [clang-format] Don't misannotate in CTor init list

owenpan accepted this revision. owenpan added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1077-1079 + EXPECT_TOKEN(Tokens[6], tok::identifier, TT_Unknown); + EXPECT_TOKEN(Tokens[7], tok::l_brace, TT

[PATCH] D135918: [clang-format] Fix lambda formatting in conditional

owenpan accepted this revision. owenpan added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:336 + Previous.MatchingParen && Previous.MatchingParen->Previous && + Previous.MatchingParen->Previous->is(tok::r_brace) && + Previous.MatchingParen

[PATCH] D136658: [clang-format] Move InsertBraces unit tests out of FormatTest.cpp

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, rymiel. owenpan added a project: clang-format. Herald added a project: All. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also add line ra

[PATCH] D136437: [clang-format] Insert closing braces after an unaffected line

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG37e754e5801c: [clang-format] Insert closing braces after an unaffected line (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D136437: [clang-format] Insert closing braces after an unaffected line

owenpan updated this revision to Diff 469969. owenpan added a comment. This revision is now accepted and ready to land. Fixed assertion failures on the tests in https://github.com/llvm/llvm-project/issues/58161#issuecomment-1287904972. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13643

[PATCH] D136437: [clang-format] Insert closing braces after an unaffected line

owenpan planned changes to this revision. owenpan added a comment. The assertions failed on the new examples in #58161. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13643

[PATCH] D136437: [clang-format] Insert closing braces after an unaffected line

owenpan added a comment. In D136437#3874832 , @MyDeveloperDay wrote: > Does this need a unit test? or are we good? I will add the tests from https://github.com/llvm/llvm-project/issues/58161 in another patch after moving InsertBraces tests out of Forma

[PATCH] D136336: [clang-format] Mark pragma region lines as StringLiterals

owenpan added a comment. Can you add a test to `TokenAnnotatorTest.cpp`? Comment at: clang/unittests/Format/FormatTest.cpp:19973-19974 + verifyFormat("#pragma region TEST(FOO: NOSPACE)", Style); + EXPECT_EQ("#pragma region TEST(FOO: NOSPACE)", +format("#pragma reg

[PATCH] D136154: [clang-format] Fix the continuation indenter

owenpan added a comment. In D136154#3867328 , @MyDeveloperDay wrote: > Can we log a GitHub issue I can’t see what you are trying to fix @hel-ableton Can you log an issue for this and add a link to it in `SUMMARY`? In D136154#3867441

[PATCH] D136437: [clang-format] Insert closing braces of unaffected lines

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, rymiel. owenpan added a project: clang-format. Herald added a project: All. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The closing brac

[PATCH] D135972: [clang-format] Don't crash on malformed preprocessor conditions

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1216 + // Don't crash when there is an #else without an #if. + if (PPBranchLevel <= -1) { +conditionalCompilationStart(/*Unreachable=*/true); Nit. Comment

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

owenpan added a comment. In D129443#3867364 , @rymiel wrote: > How should I proceed with a stale rejecting review? You can commit as https://reviews.llvm.org/D129443#3641571 has been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D134853: [clang-format] Correctly annotate UDLs as OverloadedOperator

owenpan accepted this revision. owenpan added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134853/new/ https://reviews.llvm.org/D134853 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D134853: [clang-format] Correctly annotate UDLs as OverloadedOperator

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2126 return false; FormatToken *LeftOfParens = Tok.MatchingParen->getPreviousNonComment(); Perhaps add: ``` if (Tok.MatchingParen->is(TT_OverloadedOperatorLParen))

[PATCH] D134853: [clang-format] Correctly annotate UDLs as OverloadedOperator

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1186-1199 +tok::amp, tok::ampamp)) { CurrentToken->Previous->setType(TT_OverloadedOperator); } +// User defined literal without a

[PATCH] D135918: [clang-format] Fix lambda formatting in conditional

owenpan added a comment. In D135918#3857711 , @HazardyKnusperkeks wrote: > Fun fact, there is one instance of such a case within the clang-format code. > I can't remember where (I stumbled upon this in January, and tried to fix it > ever since), should

[PATCH] D135871: [clang-format][NFC] Handle language specific stuff at the top...

owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Format/TokenAnnotator.cpp:3121 + return 1; +if (Right.is(tok::period)) + return 500; HazardyKnusperkeks wrote: > o

[PATCH] D135871: [clang-format][NFC] Handle language specific stuff at the top...

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3121 + return 1; +if (Right.is(tok::period)) + return 500; Otherwise, it wouldn't be NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D135707: [clang-format] Correctly annotate star/amp in function pointer params

owenpan added a comment. In D135707#3858097 , @MyDeveloperDay wrote: > Thanks @rymiel its great to have another active contributor, especially one > who seems so focused on github issues. Really appreciate your recent > contributions. Can we start incl

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

owenpan added a comment. In D129443#3857608 , @rymiel wrote: > Changing the default LLVM style would change the output of all the > requires-related test cases in `FormatTest.Concepts`. Should I change these > test cases to use the new indentation or pa

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

owenpan added inline comments. Comment at: clang/lib/Format/Format.cpp:1304 LLVMStyle.RequiresClausePosition = FormatStyle::RCPS_OwnLine; + LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_Keyword; LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;

[PATCH] D135866: [clang-format][NFC] Fix comment grammer in ContinuationIndenter

owenpan accepted this revision. owenpan 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/D135866/new/ https://reviews.llvm.org/D135866 _

[PATCH] D135872: [clang-format] Handle unions like structs and classes

owenpan accepted this revision. owenpan 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/D135872/new/ https://reviews.llvm.org/D135872 _

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

owenpan added inline comments. Comment at: clang/docs/ReleaseNotes.rst:540 -- Add `RemoveSemicolon` option for removing `;` after a non-empty function definition. +- Add ``RemoveSemicolon`` option for removing ``;`` after a non-empty function definition. +- Add ``

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

owenpan added a comment. In D129443#3847319 , @rymiel wrote: > Note that I didn't change the LLVM config, which right now is still set to > `Keyword`. We should change it back to `OuterScope`. Comment at: clang/lib/Format/Format.cpp:

[PATCH] D135422: [clang-format] Fix misattributing preprocessor directives to macros

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb0758f62afb6: [clang-format] Fix mis-attributing preprocessor directives to macros (authored by jacob-abraham, committed by owenpan). Changed prior

[PATCH] D135422: [clang-format] Fix misattributing preprocessor directives to macros

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:654 break; + if(Line->InMacroBody != InMacroBody) +break; Did you run `git-clang-format`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135422/n

[PATCH] D135422: [clang-format] Fix misattributing preprocessor directives to macros

owenpan accepted this revision. owenpan added a comment. LGTM. In D135422#3847053 , @jacob-abraham wrote: > I do not have commit access, can someone make this commit on my behalf? Thank > you! We need your name and email address for the authorship.

[PATCH] D135422: [clang-format] Fix misattributing preprocessor directives to macros

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2986-2990 + verifyFormat("#define X " + "\\\n" + " case 0: break;\n" + "#include \"f\

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

owenpan accepted this revision. owenpan added a comment. LGTM Comment at: clang/unittests/Format/FormatTest.cpp:26773 + Style); + + // These tests are here to show a problem that may not be easily owenpan wrote: > Add the test case if it will pas

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

owenpan added a comment. Since this option doesn't work for empty functions yet, we should either add a note in the documentation or qualify the scope with "non-empty" as suggested in my comments. Comment at: clang/docs/ReleaseNotes.rst:528 +- Add `RemoveSemicol

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:963 nextToken(/*LevelDifference=*/-AddLevels); HandleVerilogBlockLabel(); lahwaacz wrote: > owenpan wrote: > > The `while` loop would address > > https://reviews.llvm.org

<    1   2   3   4   5   6   7   8   9   10   >