[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-03-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D42684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-03-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-03-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-03-05 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-03-02 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Also double-checked with Richard Smith and he agrees that the current > behavior is preferable. For comma and plus this doesn't seem overly > important, but making it: > > aa(b + ccc * > d); > > > seems

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-03-02 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. If people don't care about this case, we might as well merge this :-) Just kidding. The tweak matches both our expectation, the auto-indent behaviour of IDE (not to be trusted, but still probably of 'default' behaviour for many people, esp. when you don't yet use a

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-03-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 136557. Typz added a comment. Change options values to No/MultiLine/Yes. Repository: rC Clang https://reviews.llvm.org/D42684 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-03-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-03-01 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC326426: [clang-format] Add SpaceBeforeColon option (authored by Typz, committed by ). Changed prior to commit: https://reviews.llvm.org/D32525?vs=136305=136483#toc Repository: rC Clang

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-03-01 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL326426: [clang-format] Add SpaceBeforeColon option (authored by Typz, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D32525 Files:

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42787#1022081, @djasper wrote: > But you *do* want extra indentation in the case of: > > function(a, >b + >cc); > > > I understand you argument, but I don't agree at the moment. As is (without > getting

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Indeed, seems to apply to classes as well. Maybe I was mislead by my testing, where I did not get the case (possibly because we use `ConstructorInitializerAllOnOneLineOrOnePerLine=true`, so the continuation indenter only sees "short" class declarations unless breaking the

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 136306. Typz added a comment. Address review comments of https://reviews.llvm.org/D32525 Repository: rC Clang https://reviews.llvm.org/D43015 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: unittests/Format/FormatTest.cpp:8969 + "barr(1) {}", CtorInitializerStyle); + CtorInitializerStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma; +

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 136305. Typz marked 2 inline comments as done. Typz added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D32525 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 136300. Typz marked an inline comment as done. Typz added a comment. Address review comments, and rebase on https://reviews.llvm.org/D32525 Repository: rC Clang https://reviews.llvm.org/D43015 Files: docs/ClangFormatStyleOptions.rst

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 136299. Typz marked 2 inline comments as done. Typz added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D32525 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. The problem I have is really related to the current `AlwaysBreakTemplateDeclarations` behavior, which does not apply to functions. I set it to false, and I get this: template<> void ( const

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D32525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Some initial design work has been done, and Krasimir said that he's > interested. No timeline though :( any update or progress maybe? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42684#1013005, @djasper wrote: > Please given an explanation of what you are trying to achieve with this > change. Do you intend to set the penalty high so that clang-format does other > things first before falling back to wrapping template

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42729#994841, @djasper wrote: > - Of course you find all sorts of errors while testing clang-format on a > large-enough codebase. That doesn't mean that users run into them much. > - We have had about 10k clang-format users internally for

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42787#994781, @djasper wrote: > What I mean is, users will find it surprising if whether or not a parameter > gets wrapped leads to a different indentation internal to that parameter. I > have not heard of a single user that would be surprised

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > I don't agree that that's the same thing. The closing brace is still neatly > aligned with the line of the opening brace (which happens to be just the > opening brace). This invariant is not really applicable to switch statements, where code of each "branch" is already

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-27 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2183 return 0; +if (Left.Previous && Left.Previous->is(tok::equal) && +!Style.Cpp11BracedListStyle) djasper wrote: > Why is this necessary? Otherwise the

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-27 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 136081. Typz added a comment. Prevent breaking between = and { when Cpp11BracedListStyle=false. Repository: rC Clang https://reviews.llvm.org/D43290 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index:

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-27 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. >> Tweaking the penalty handling would still be needed in any case, since the >> problem happens also when Cpp11BracedListStyle is true (though the effect is >> more subtle) > > I don't understand this. Which style do you actually care about? With >

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-27 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL326191: clang-format: fix formatting of ObjC @synchronized blocks (authored by Typz, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-27 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL326192: clang-format: use AfterControlStatement to format ObjC control blocks (authored by Typz, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 134641. Typz added a comment. Fix bug which was lingering: prevent inlining of ObjC special control blocks. Repository: rC Clang https://reviews.llvm.org/D43232 Files: include/clang/Format/Format.h lib/Format/UnwrappedLineFormatter.cpp

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked an inline comment as done. Typz added inline comments. Comment at: unittests/Format/FormatTestObjC.cpp:193-198 + verifyFormat("@synchronized(self) {\n" + " f();\n" + "}\n" + "@synchronized(self) {\n" + "

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I'll try to fix this. Can you please review D43114 in the mean time? This patch depends on it, so it must be merged first. Repository: rC Clang https://reviews.llvm.org/D43232 ___

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. A user can create an error by reasoning like this, after the code has been formatted this way (a long time ago) : "oh, I need to make an extra function call, but in all cases ah, here is the end of the switch, let's put my function call here". I am not saying it

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D43232#1008004, @benhamilton wrote: > Thanks! Can you add a test for this, please? There are actually tests, though they are performed by activating Allman brace wrapping mode. I am trying to change them, but this highlights another issue:

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D43183#1008632, @djasper wrote: > Yes, that's what I mean. What do you mean, the style is too error prone? When `IndentCaseLabels` is false, the code gets formatted in a way that "hides" the structure of the code, by indenting the end of the

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 134411. Typz marked an inline comment as done. Typz added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D43290 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index:

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Tweaking the penalty handling would still be needed in any case, since the problem happens also when Cpp11BracedListStyle is true (though the effect is more subtle) Generally, I think it is better to just rely on penalties, since it gives a way to compare and weight each

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-14 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. This patch changes the behavior of PenaltyBreakBeforeFirstCallParameter so that is does not apply when the brace comes after an assignment. This way, variable initialization is wrapped more like an initializer than like

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Hum, not sure I fully got your proposal. So you actually mean that clang-format you format like this (with 4-space indent for clarity): switch (x) { case 0: break; case 1: { foo(); break; } case 2: { foo(); } break; case 3: {

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. It is explicitly documented in google style guide: https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements : > case blocks in switch statements can have curly braces or not, depending on > your preference. If you do include curly braces they should

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > We'll just format those cases in a somewhat weird way and users can either > accept that or change their code to not need it. I think we have a really diverging opinion on this. From my experience, people will mostly accept the output of the formatter without question :

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 4 inline comments as done. Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine();

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. This is done as discussed in https://reviews.llvm.org/D43114. But I can instead add a new `AfterObjCSpecialBlock` brace wrapping flag. Repository: rC Clang https://reviews.llvm.org/D43232 ___ cfe-commits mailing list

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek, benhamilton. ObjC defines `@autoreleasepool` and `@synchronized` control blocks. These used to be formatted according to the `AfterObjCDeclaration` brace- wrapping flag, which is not very consistent. This patch changes

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Do you have a reference to style guides recommending any of this? Unfortunately not... I think this is a really advanced topic, and often not recommended way to format code at all (e.g. "if you need a block, you may as well use a function"). I can find the current

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. When a block is started after a case label, clang-format does add extra indent to the content of this block: the block content is indented one level (with respect to the switch) while the closing brace is not indented,

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine(); benhamilton wrote: > Typz wrote: > >

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC324741: clang-format: keep ObjC colon alignment with short object name (authored by Typz, committed by ). Changed prior to commit: https://reviews.llvm.org/D43121?vs=133619=133620#toc Repository: rC

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133619. Typz added a comment. rebase on latest master. Repository: rC Clang https://reviews.llvm.org/D43121 Files: lib/Format/ContinuationIndenter.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index:

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:900 + std::max(NextNonComment->LongestObjCSelectorName, + unsigned(NextNonComment->TokenText.size())) - NextNonComment->ColumnWidth;

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133596. Typz marked 2 inline comments as done. Typz added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D43121 Files: lib/Format/ContinuationIndenter.cpp lib/Format/TokenAnnotator.cpp

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133589. Typz added a comment. fix type in commit message Repository: rC Clang https://reviews.llvm.org/D43121 Files: lib/Format/ContinuationIndenter.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index:

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. When the target object expression is short and the first selector name is long, clang-format used to break the colon alignment: [I performSelectorOnMainThread:@selector(loadAccessories)

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42787#1000687, @krasimir wrote: > We could adapt the single-argument version instead, turning: > > foo(bb + > c); > > > into: > > foo(bb + > c); > We could

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D42684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine(); Wondering if formatting with this style is

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. The blocks used to be formatted using the "default" behavior, and would thus be mistaken for function calls followed by blocks: this could lead to unexpected inlining of the block and extra line-break before the opening

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-07 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133204. Typz added a comment. Fix indentation of inheritance list, which is actually based on `ConstructorInitializerIndentWidth`. Maybe a new option (`InheritanceListIndentWidth`) should be used instead? Repository: rC Clang

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-07 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: djasper, krasimir, klimek. This option replaces the BreakBeforeInheritanceComma option with an enum, thus introducing a mode where the colon stays on the same line as constructor declaration: // When it fits on line: class A : public B,

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Indeed, I have yet find more precisely documented coding rules which require this format, but I thought I could at least address the non-precise aspect of the patch itself in the mean-time. https://reviews.llvm.org/D32525

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132963. Typz added a comment. Split the option into 3 separate options: SpaceBeforeCtorInitializerColon, SpaceBeforeInheritanceColon and SpaceBeforeRangeBasedForLoopColon. This makes each option clearer and more consistent, with no ambiguities due to

[PATCH] D37813: clang-format: better handle namespace macros

2018-02-05 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132838. Typz added a comment. rebase https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp

[PATCH] D33440: clang-format: better handle statement macros

2018-02-05 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132837. Typz added a comment. Use StatementMacro detection to improve brace type detection heuristics (in UnwrappedLineParser::calculateBraceTypes). https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-02-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > I don't think cases where there is only a semicolon-less macro are > particularly frequent/important either. You probably could even add a > semicolon in those cases, right? How frequent are such cases in your > codebase? Either way, they aren't fixed by this patch, so

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > You might doubt it, but having written the code I can tell you that it's the > case. Ok, you win :-) > I see the argument why this indentation is not necessary in exactly the case > where the last parameter is multi-line and not wrapped to a new line itself: > You

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I doubt this particular was intentional, esp. since this case never happens in the tests. I think it is more a side-effect of the (general) indent in "fake" parenthesis. Here is an exemple: Before this change: foo(a, bb +

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. There should be no extra indent when wrapping only the expression used as last argument. This is consistent with the behavior when the first (and only) argument's expression is wrapped. foo(a, bb +

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-02-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132341. Typz added a comment. If the template declaration spans multiple lines, force wrap before the function/class declaration Repository: rC Clang https://reviews.llvm.org/D42684 Files: include/clang/Format/Format.h

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132198. Typz added a comment. Force wrap after multi-line template declaration Repository: rC Clang https://reviews.llvm.org/D42684 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42729#993159, @djasper wrote: > I think this case is not important enough to fix. Please tell users to just > delete the useless semicolon. I would agree if were simple to spot: but often this may manifest itself only with a missing space

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. There are actually other cases, e.g. with macros "containing" the semicolon: void abort() { FOO() BAR() }; gets reformatted to this (still wrong with this patch, but the space after the parenthesis is added): void abort(){ FOO() BAR() }; And also this one

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1911 + if (Next && Next->is(tok::l_brace) && Next->BlockKind == BK_BracedInit) + Next->BlockKind = BK_Block; +} this may actually not be enough in all cases: to completely

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132130. Typz added a comment. update commit message Repository: rC Clang https://reviews.llvm.org/D42729 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp

[PATCH] D42729: clang-format: Fix formatting of function blocks followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. In some case, the heuristic used to identify the type of blocks (in UnwrappedLineParser) mistakens a Block for a BracedInit, when the TokenAnnotator eventually finds (correclty) that this is a function declaration. This

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-01-30 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 131946. Typz added a comment. fix commit message Repository: rC Clang https://reviews.llvm.org/D42684 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-01-30 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. Introduce ``PenaltyBreakTemplateDeclaration`` to control the penalty, and change ``AlwaysBreakTemplateDeclarations`` to an enum with 3 modes: - ``None`` for automatic (e.g. penalty based) wrapping of template

[PATCH] D37813: clang-format: better handle namespace macros

2017-12-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. OK. So you mean a solution like the one discussed earlier would be the way to go? > I mean that we can configure macros in the format style, like "define A(X) > class X {". I'm not 100% sure whether we would just try to use the > Preprocessor for this, or whether we'd

[PATCH] D33440: clang-format: better handle statement macros

2017-12-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D33440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

2017-12-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

2017-12-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Ok, that's probably where our different opinions come from - I would want a > macro to be formatted to reflect how it's implemented, because otherwise I'm > going to be surprised when I look at the implementation, and I consider > surprises to be something to avoid in

[PATCH] D37813: clang-format: better handle namespace macros

2017-12-05 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I don't think this is really relevant for this tool: if someone changes the implementation of the macro, then *they* must indeed if it should not be formatted like a namespace (and keep the clang-format configuration unchanged), or if it should now be formatted like a

[PATCH] D37813: clang-format: better handle namespace macros

2017-12-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. As far as "parsing" and formatting inside the block is concerned, this is indeed unrelated (and would totally work if all macros where specified with some preprocessor definitions). But identifying the 'opening' token and generating the matching 'closing' comment are

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-12-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Indeed, looks good, thanks! Though that exacerbates the alignment issue, I now get things like this: enum { SomeLongerEnum, // comment SomeThing, // comment Foo, // something } ^ (column limit) The comment on 'Foo' would

[PATCH] D37813: clang-format: better handle namespace macros

2017-12-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Definitely that would be much more expressive. But this approach is also incomplete: in case of namespace (and possibly others?), we also need to perform the reverse operation, e.g. to "generate" a macro call for rewriting the closing comment. On top of this, I think

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-12-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I think the difference between code and comments is that code "words" are easily 10 characters or more, whereas actual words (in comments) are very often less than 10 characters: so code overflowing by 10 characters is not very frequent. whereas small words in comment

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-11-23 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > @klimek wrote: > In the above example, we add 3 line breaks, and we'd add 1 (or more) > additional line breaks when reflowing below the column limit. > I agree that that can lead to different overall outcomes, but I don't see > how the approach of this patch really

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-11-21 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Btw, another issue I am having is that reflowing does not respect the alignment. For exemple: enum { Foo,///< This is a very long comment Bar,///< This is shorter BarBar, ///< This is shorter } Stuff; will be reflown to : enum { Foo,

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-11-21 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33589#925903, @klimek wrote: > I think this patch doesn't handle a couple of cases that I'd like to see > handled. A counter-proposal with different trade-offs is in > https://reviews.llvm.org/D40068. It may be simpler (though not to my

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-11-21 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33589#924716, @klimek wrote: > One interesting trade-off I'm running into: > My gut feeling is that we really want to make local decisions about whether > we want to break/reflow - this makes the code significantly simpler (IMO), > and

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

2017-11-21 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Generally, this indeed improves the situation (though I cannot say much about the code itself, it is still too subtle for my shallow knowledge of clang-format). But it seems to give some strange looking result with long comments: it seems like the decision is made at

[PATCH] D37813: clang-format: better handle namespace macros

2017-11-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement macros

2017-11-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D33440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement macros

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 18. Typz marked 5 inline comments as done. Typz added a comment. Address review comments https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp

[PATCH] D33440: clang-format: better handle statement macros

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33440#920205, @djasper wrote: > Out of curiosity, will this be able to fix the two situations that you get > for python extension? > There, you usually have a PyObject_HEAD with out semicolon in a struct and > than a PyObject_HEAD_INIT(..) in

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Unless I'm missing something, I'd agree with Daniel; this is not a rule > that's widely used, and I'd say reformatting a code base to the > clang-formatted variant will not regress readability. Unfortunately coding rules are not just about readability, but also about

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Sorry for the long response time. I don't see this style rule expressed > explicitly in the Skia or QtCreator style guides - is this something that > just happens to be done sometimes in the code bases? This is present in code base. Those project's style guides are not

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D32478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement macros

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D33440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-11-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping ? https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

<    1   2   3   4   >