[PATCH] D134329: [clang-format][NFC] Reformat clang/lib/Format using 6257832bf94f

owenpan added a comment. In D134329#3822028 , @MyDeveloperDay wrote: > I guess the clang-format check might come before any building of the code. +1. > You know if we relex the "Unknown" YAML option, this would go a long way not > only to help us here

[PATCH] D134733: [clang-format][chore] transparent #include name regex

owenpan added a comment. While attempting to review this patch, I came to the conclusion that we should first clean up the `HeaderIncludes` class and Format.cpp a little. I will submit a patch for review ASAP. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, kwk, HazardyKnusperkeks, curdeius. 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. Repositor

[PATCH] D134733: [clang-format][chore] transparent #include name regex

owenpan added a project: clang-format. owenpan added a comment. See D134852 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134733/new/ https://reviews.llvm.org/D134733

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

owenpan added a comment. @kwk D134733 reminded me of https://reviews.llvm.org/D121370#3401173, which I realized didn't go far enough. This NFC patch is what I should have suggested to you then. If you want to do the cleanup differently, you can either make sug

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

owenpan added inline comments. Comment at: clang/lib/Format/Format.cpp:2774 - -const char CppIncludeRegexPattern[] = -R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; kwk wrote: > owenpan wrote: > > kwk wrote: > > > MyDeveloperDay wrote: > > > > D

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

owenpan added a comment. In D134853#3822842 , @rymiel wrote: > Unless any reviewers have any other opinions, I would leave fixing those out > of this patch and leave the tests "incomplete" for now? +1. > Also, UDLs that don't start with an underscore a

[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

owenpan added a comment. I'm not against extending clang-format to support yet another language like Carbon, but we should probably wait until it has gained widespread usage? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134652/new/ https://reviews.llvm.org/D134652 ___

[PATCH] D135026: [clang-format] Handle C# interpolated verbatim string prefix @$

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, curdeius. 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://

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

owenpan added a comment. In D134853#3829433 , @rymiel wrote: > So, after some investigation, it turns out the clang lexer only combines an > underscore-less UDL to a single `string_literal` token if it's a valid > standard library UDL (in the given stan

[PATCH] D135026: [clang-format] Handle C# interpolated verbatim string prefix @$

owenpan added inline comments. Comment at: clang/lib/Format/.clang-format:2 BasedOnStyle: LLVM -InsertBraces: true +#InsertBraces: true RemoveBracesLLVM: true Commented out by mistake. Will undo it when landing. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D135026: [clang-format] Handle C# interpolated verbatim string prefix @$

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb60e7a7f1afc: [clang-format] Handle C# interpolated verbatim string prefix @$ (authored by owenpan). Changed prior to commit: https://reviews.llvm

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7cbc9206697e: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES

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

owenpan added a comment. Is this option really required? Can we just fix the regression as reported in https://github.com/llvm/llvm-project/issues/56283? It seems that we haven't followed the policy late

[PATCH] D134733: [clang-format][NFC] more centric handling of include name matching

owenpan added a comment. Can you rebase? I'll finish the review after that. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:410 +llvm::StringRef getIncludeNameFromMatches( +const llvm::SmallVectorImpl &Matches) { + if (Matches.size() >= 3) { Us

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

owenpan added a comment. @MyDeveloperDay Cool! In D135466#3843792 , @HazardyKnusperkeks wrote: > Maybe even extend the option to other unnecessary semicolons like `int x = > 5;;` or other noop statements, one just has to be careful not to remove the >

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

owenpan added a comment. In D129443#3842427 , @HazardyKnusperkeks wrote: > We can //fix// that regression, and I will adopt this change for my company > to keep the requires expressions where they are. :) Then we have three options: 1. Only fix the "r

[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

[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 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] 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] 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/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

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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D139801: [clang-format] Adds 'friend' to QualifierOrder.

owenpan added a comment. In D139801#3991108 , @MyDeveloperDay wrote: > I also appreciate the concessions made by others to allow clang-format to > alter code as this has opened the doors in my view to 1 or 2 other great code > modification changes. +1

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4cafc3727b35: [clang-format] Add 'friend' to QualifierOrder (authored by red1bluelost, committed by owenpan). Repository: rG LLVM Github Monorepo

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

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. Closes https://g

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan planned changes to this revision. owenpan added a comment. Will add unit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 ___ cfe-commits mailing

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 484801. owenpan added a comment. Fixes a couple of bugs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Fo

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 484841. owenpan added a comment. Fixed another bug and added some unit tests. Also updated ReleaseNotes.rst and addressed some review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 Files: cla

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan marked an inline comment as done. owenpan added inline comments. Comment at: clang/include/clang/Format/Format.h:2395 + /// >0: Insert separators between digits, starting from the rightmost digit. + struct IntegerLiteralSeparatorStyle { +/// \code H

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan planned changes to this revision. owenpan added a comment. Will extend this option to support C#, Java, and JavaScript using the underscore character `_` as the separator. Comment at: clang/unittests/Format/FormatTest.cpp:25124 +TEST_F(FormatTest, IntegerLiteralSepar

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 485180. owenpan added a comment. This revision is now accepted and ready to land. - Adds support for C#, Java, and JavaScript using `_`. - Adds support for only formatting affected ranges. - Simplifies ``IntegerLiteralSeparatorFixer::format()``. - Adds more as

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan planned changes to this revision. owenpan added a comment. Will add more unit tests and perhaps move them to their own file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 ___ cfe-commits ma

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 485188. owenpan added a comment. This revision is now accepted and ready to land. - Fixed an assertion failure. - Added unit tests . - Moved all unit tests to another file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.ll

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 485210. owenpan added a comment. Cleaned up the unit tests a little bit: - Made them a little more varied. - Removed some overlapping test cases. - Removed integer literal suffixes for JavaScript. - Added an octal `BigInt` test case for JavaScript. CHANGES

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG46c94e5067b5: [clang-format] Add an option to format integer literal separators (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan added a comment. In D140543#4016156 , @thakis wrote: > This seems to break tests everywhere, eg > http://45.33.8.238/linux/95289/step_7.txt > > Please take a look and revert for now if it takes a while to fix. I had run FormatTests on Windows an

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 485600. owenpan added a comment. Fix a memory bug, which doesn't surface in Debug build. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/Release

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan requested review of this revision. owenpan added a comment. In D140543#4016184 , @thakis wrote: > In D140543#4016181 , @owenpan wrote: > >> In D140543#4016156 , @t

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan updated this revision to Diff 485658. owenpan added a comment. It turns out that the memory bug had nothing to do with whether to use the stack or heap memory for the `Token` object. The culprit was passing `getFormattingLangOpts(Style)` directly to the `Lexer` constructor. I suppose in

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

owenpan requested review of this revision. owenpan added inline comments. Comment at: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp:81-82 + + llvm::SpecificBumpPtrAllocator Allocator; + auto Tok = new (Allocator.Allocate()) Token; + Lex->LexFromRawLexer(*Tok);

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG89aad1e6a397: Reland [clang-format] Add an option to format integer literal separators (authored by owenpan). Repository: rG LLVM Github Monorepo

[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

owenpan created this revision. owenpan added reviewers: 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-commits. Account for an r

[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

owenpan accepted this revision. owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:23593-23594 - "template \n" - "concept DelayedCheck = static_cast(0) ||\n" - " requires(T t) { t.bar(); } && sizeof(T) <= 8;");

[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

owenpan added a comment. In D140835#4023080 , @HazardyKnusperkeks wrote: > On second thought, shouldn't we test for removing the braces? Hmm. This patch including the added test case //is// for removing braces. Repository: rG LLVM Github Monorepo C

[PATCH] D140956: [clang-format] Add an option for breaking after C++ 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. This patch only

[PATCH] D140956: [clang-format] Add an option for breaking after C++ attributes

owenpan added inline comments. Comment at: clang/lib/Format/FormatToken.h:324 + /// \c true if this token starts a C++ attribute. + unsigned StartsCppAttribute : 1; + Can be removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

owenpan updated this revision to Diff 486445. owenpan retitled this revision from "[clang-format] Add an option for breaking after C++ attributes" to "[clang-format] Add an option for breaking after C++11 attributes". owenpan edited the summary of this revision. owenpan added a comment. - Delete

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

owenpan added a comment. In D140956#4025614 , @MyDeveloperDay wrote: > Should we care about other forms of attributes like `__stdcall` etc..? I > personally don't, just I assume from the tests this isn't covered (maybe just > add that limitation to the

[PATCH] D141035: [clang-format] Add an option to insert a newline at EOF if missing

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://g

[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

owenpan added a comment. In D140835#4028257 , @HazardyKnusperkeks wrote: > In D140835#4023132 , @owenpan wrote: > >> In D140835#4023080 , >> @HazardyKnusperkeks wrote: >

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

owenpan added a comment. In D140956#4028147 , @MyDeveloperDay wrote: > If someone wants to extend this to include the old form, then that can be a > completely different review. Better yet, use another tool like clang-tidy to replace the old with the n

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa28f0747c2f3: [clang-format] Add an option for breaking after C++11 attributes (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D140843: [clang-format] fix template closer followed by >

owenpan added a comment. In D140843#4028142 , @MyDeveloperDay wrote: > I'd like one of @owenpan, @HazardyKnusperkeks or @rymiel to comment before > commit, just to get another view. Perhaps clang-format should never insert a space in `>>`. CHANGES S

[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

This revision was automatically updated to reflect the committed changes. Closed by commit rG5751c439be7d: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine() (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140835/new

[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

owenpan added a comment. In D140835#4028479 , @HazardyKnusperkeks wrote: > In D140835#4028274 , @owenpan wrote: > >> If `RemoveBracesLLVM` is false, the line is too long and will wrap. > > That's what I fail to s

[PATCH] D141035: [clang-format] Add an option to insert a newline at EOF if missing

owenpan added a comment. In D141035#4028134 , @MyDeveloperDay wrote: > I think I've died an gone to heaven!! LGTM... Happy New Year! Happy New Year to you too! Thanks to your comment in D19031#1747112 , I remem

[PATCH] D141035: [clang-format] Add an option to insert a newline at EOF if missing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2c6ecc9db624: [clang-format] Add an option to insert a newline at EOF if missing (authored by owenpan). Changed prior to commit: https://reviews.l

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

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. To prevent `\r\n

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

owenpan added a comment. In D141098#4030426 , @rymiel wrote: > The LLVM Coding Standard apparently doesn't mention line endings..? Line endings probably should never be specified in coding standards, but the default should match the majority, and in the

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

owenpan added a comment. In D141098#4032951 , @rymiel wrote: >> I think we should combine `DeriveLineEnding` and `UseCRLF` > > Do you mean a single `LineEnding` options which is an enum with `LF`, `CRLF` > and `Derive`, or similar? Yep. Repository:

[PATCH] D141288: [clang-format] Inherit RightAlign options across scopes

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/D141288/new/ https://reviews.llvm.org/D141288 _

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

owenpan added a comment. In D140956#4038988 , @krasimir wrote: > It looks like this regressed the following example: > > % cat test.cc # formatted with older clang-format > a > &cc() {

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

owenpan added a comment. In D140956#4042990 , @krasimir wrote: >> Was it actually a regression or did this patch also fix a bug? It seems that >> the continuation indent before the & in your example is inconsistent with >> other similar function declara

[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] 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] 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] 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] 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] 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] 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] 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" goldstein.w.n wrote

[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] 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] 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] 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 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] 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] 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] 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. In D137181#3911005 , @sstwcw wrote: > Here is one problem: > > clang-format -style='{IndentPPDirectives: BeforeHash, PPIndentWidth: 1, > IndentWidth: 4, IndentCaseLabels: true}' > > actual: > #define X

[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] 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] 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] 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. 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. 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 (

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