djasper (Daniel Jasper)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:15 AM (293 w, 4 d)

Recent Activity

Yesterday

djasper committed rL326024: Remove unused variable. We should be warning-free..
Remove unused variable. We should be warning-free.
Fri, Feb 23, 11:00 PM
djasper committed rC326024: Remove unused variable. We should be warning-free..
Remove unused variable. We should be warning-free.
Fri, Feb 23, 10:59 PM
djasper closed D43673: Make module use diagnostics refer to the top-level module.

Submitted as r326023.

Fri, Feb 23, 10:56 PM
djasper committed rC326023: Make module use diagnostics refer to the top-level module.
Make module use diagnostics refer to the top-level module
Fri, Feb 23, 10:56 PM
djasper committed rL326023: Make module use diagnostics refer to the top-level module.
Make module use diagnostics refer to the top-level module
Fri, Feb 23, 10:56 PM
djasper created D43673: Make module use diagnostics refer to the top-level module.
Fri, Feb 23, 5:25 AM

Wed, Feb 21

djasper accepted D43598: [clang-format] Tidy up new API guessLanguage().

Thank you for doing these follow up changes!

Wed, Feb 21, 3:38 PM
djasper added inline comments to D43522: [clang-format] New API guessLanguage().
Wed, Feb 21, 3:20 PM
djasper added inline comments to D43522: [clang-format] New API guessLanguage().
Wed, Feb 21, 9:45 AM

Tue, Feb 20

djasper added a comment to D43290: clang-format: tweak formatting of variable initialization blocks.

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)

Tue, Feb 20, 11:10 PM
djasper added a comment to D42684: clang-format: Allow optimizer to break template declaration..

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 declarations?

Tue, Feb 20, 2:29 AM

Mon, Feb 19

djasper accepted D43440: clang-format: [JS] fix `of` detection..

Looks good.

Mon, Feb 19, 4:10 AM

Thu, Feb 15

djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

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".

Thu, Feb 15, 8:12 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

That doesn't explain to me how this is error prone. I can't think how you'd create an error by this that would not be caught by the compiler. Much less if you consistently use clang-format.

Thu, Feb 15, 7:04 AM
djasper accepted D43298: [clang-format] Support repeated field lists in protos.

Looks good.

Thu, Feb 15, 5:34 AM
djasper edited reviewers for D43312: [clang-format] fix handling of consecutive unary operators, added: krasimir; removed: djasper.
Thu, Feb 15, 4:52 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

Yes, that's what I mean. What do you mean, the style is too error prone?

Thu, Feb 15, 4:51 AM
djasper accepted D43303: [Format] Fix for bug 35641.

Thanks for the fix.

Thu, Feb 15, 4:32 AM · Restricted Project

Wed, Feb 14

djasper added a comment to D43290: clang-format: tweak formatting of variable initialization blocks.

What you are doing makes sense to me. My only hesitation is whether we should maybe completely disallow breaking between = and { when Cpp11BracedListStyle is false instead of solving this via penalties. Specifically,

Wed, Feb 14, 9:33 AM
djasper accepted D43294: [clang-format] Recognize percents as format specifiers in protos.

Ok.. I guess ;)

Wed, Feb 14, 9:26 AM

Tue, Feb 13

djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

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 be placed as shown below.

If not conditional on an enumerated value, switch statements should always have a default case (in the case of an enumerated value, the compiler will warn you if any values are not handled). If the default case should never execute, simply assert:

switch (var) {
  case 0: {  // 2 space indent
    ...      // 4 space indent
    break;
  }
  case 1: {
    ...
    break;
  }
  default: {
    assert(false);
  }
}

So IMHO we cannot just change the current (or default) behaviour.

Tue, Feb 13, 7:51 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

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 : therefore I think we should strive to have the formatter format things as "correctly" as possible. And looking at the existing options and various complex formatting algorithms implemented I think this reasoning has been applied to some extent :-)
So I personnally would be willing to add some code/options even for such kind of corner cases; but then I am not the one maintaining the clang-format project.

Tue, Feb 13, 6:36 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

To me none of these options make sense. For the case you are describing, I agree that the current behavior is not ideal, but neither are any of the alternatives. However, I think that's fine. 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 don't particularly care which of these options we go with, but I would be really hesitant to introduce an style flag for it. This is so rare, that almost no one needs this flag (or should need this flag). Thus, the cost of the flag (discoverability of flags for users, increased maintenance cost, etc.) strongly outweigh the benefit. IMO, for the same reason, this specific issue will not become part of the style guide of projects.

Tue, Feb 13, 5:46 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

Do you have a reference to style guides recommending any of this?

Tue, Feb 13, 1:49 AM

Mon, Feb 12

djasper accepted D43180: [clang-format] Support text proto extensions.

Cool, thanks.

Mon, Feb 12, 11:28 PM
djasper added inline comments to D43180: [clang-format] Support text proto extensions.
Mon, Feb 12, 7:05 AM

Fri, Feb 9

djasper accepted D43121: clang-format: keep ObjC colon alignment with short object name.

Looks good.

Fri, Feb 9, 6:30 AM
djasper edited reviewers for D43114: clang-format: fix formatting of ObjC @synchronized blocks, added: benhamilton, thakis; removed: djasper, klimek.
Fri, Feb 9, 4:59 AM
djasper added inline comments to D43121: clang-format: keep ObjC colon alignment with short object name.
Fri, Feb 9, 4:58 AM

Thu, Feb 8

djasper accepted D42957: [clang-format] Do not break before long string literals in protos.

Looks good.

Thu, Feb 8, 1:54 AM

Tue, Feb 6

djasper added a comment to D32525: [clang-format] Add SpaceBeforeColon option.

No. The reason for us generally asking for a style guide is because it unambiguously clarifies the exact style that is to be preferred. Projects that don't have a style guide written down also often do not agree on what the style should be.

Tue, Feb 6, 8:15 AM
djasper accepted D42727: [clang-format] Adds space around angle brackets in text protos.

Looks good.

Tue, Feb 6, 3:06 AM
djasper added a comment to D32525: [clang-format] Add SpaceBeforeColon option.

You still haven't addressed my comment about there not being a publicly accessible style guide recommending these.

Tue, Feb 6, 3:05 AM

Mon, Feb 5

djasper added a comment to D40988: Clang-format: add finer-grained options for putting all arguments on one line.

Generally, upload patches with the full file as diff context. Phabricator hides that by default, but enables me to expand beyond the patch regions (where it currently says "Context not available").

Mon, Feb 5, 12:08 AM

Fri, Feb 2

djasper added inline comments to D42727: [clang-format] Adds space around angle brackets in text protos.
Fri, Feb 2, 4:14 AM

Thu, Feb 1

djasper added a comment to D42729: clang-format: Fix formatting of function body followed by semicolon.
  • 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 several years. The semicolon issue comes up but really rarely and if it does, people happily fix their code not blaming clang-format.
Thu, Feb 1, 6:59 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

Ah, Manuel and Krasimir are already on this thread, maybe they can comment? I also added Chandler and Sam who I know care about formatting somewhat.

Thu, Feb 1, 6:33 AM
djasper added reviewers for D42787: clang-format: do not add extra indent when wrapping last parameter: chandlerc, sammccall.
Thu, Feb 1, 6:33 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

I don't mean trivial with a diff. 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 by this extra indentation.

Thu, Feb 1, 6:31 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

You might doubt it, but having written the code I can tell you that it's the case. Shame on me for not writing a test, though.

Thu, Feb 1, 6:13 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

I am against this change. The current behavior here is intentional and IMO more consistent. If there is more than one precedence level in a set of parentheses, we add the additional indentation. If you don't like it, surround it with extra parentheses.

Thu, Feb 1, 4:22 AM

Wed, Jan 31

djasper added a comment to D42729: clang-format: Fix formatting of function body followed by semicolon.

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 they aren't a good reason to move forward with it.

Wed, Jan 31, 8:05 AM
djasper added a comment to D42729: clang-format: Fix formatting of function body followed by semicolon.

I think this case is not important enough to fix. Please tell users to just delete the useless semicolon.

Wed, Jan 31, 5:20 AM

Tue, Jan 30

djasper edited reviewers for D42684: clang-format: Allow optimizer to break template declaration., added: sammccall; removed: djasper, klimek.
Tue, Jan 30, 6:21 AM
djasper accepted D42685: [clang-format] Adds space around braces in text protos.

Looks good.

Tue, Jan 30, 6:20 AM

Fri, Jan 26

djasper accepted D42570: clang-format: [JS] Prevent ASI before [ and (..

Makes sense.

Fri, Jan 26, 6:41 AM

Jan 24 2018

djasper accepted D42500: [clang-format] Fixes indentation of inner text proto messages.

Looks good

Jan 24 2018, 1:40 PM
djasper accepted D42373: [clang-format] Disable string literal breaking for text protos.

Change the comment and possibly also the patch description along the lines of what I have suggested. Other than that, looks good to me.

Jan 24 2018, 2:34 AM

Jan 23 2018

djasper accepted D42376: [clang-format] Ignore UnbreakableTailLength sometimes during breaking.

Happy to go forward with this. I think we might also wanna investigate whether entirely removing UnbreakableTailLength would be beneficial. I think we implemented it as an optimization, but I can actually imagine it saving much. Plus the code would be simpler and we would conserve some memory.

Jan 23 2018, 3:10 AM
djasper added inline comments to D42376: [clang-format] Ignore UnbreakableTailLength sometimes during breaking.
Jan 23 2018, 2:58 AM
djasper added a comment to D42373: [clang-format] Disable string literal breaking for text protos.

I am not sure we should actually do this. I agree that badly reflowing multiline string literals is not ideal, but neither is violating the column limit. In any case, proper reflowing would probably the best solution. How hard would it be to implement that (for proto as well as C++)?

Jan 23 2018, 12:23 AM

Jan 22 2018

djasper added inline comments to D42376: [clang-format] Ignore UnbreakableTailLength sometimes during breaking.
Jan 22 2018, 3:50 PM
djasper added a comment to D42036: [clang-format] Keep comments aligned to macros.

While I agree that there is probably a bug to fix, I don't (yet) agree with what is proposed in this patch. I think a comment in between preprocessor directives should always either:

  • Be considered part of the code in between the #-lines
  • Be considered to be commenting on the subsequent #-line
Jan 22 2018, 3:19 PM
djasper added a comment to D42372: [clang-format] Ignore UnbreakableTailLength sometimes during breaking.

I think I understand now. I think I'd prefer pulling all of the "+ UnbreakableTailLength" calculations out of getRemainingLength so that you don't have to pass around the new parameter CanBreakAfter. Instead, only add it if necessary outside of the function.

Jan 22 2018, 7:34 AM
djasper added a comment to D42372: [clang-format] Ignore UnbreakableTailLength sometimes during breaking.

I don't understand why the closing braces would count towards the string literals UnbreakableTailLength. Isn't that a bug?

Jan 22 2018, 7:09 AM

Jan 2 2018

djasper committed rL321649: Revert r321089: "[DAG] Elide overlapping store" (and subsequent fix in r321204).
Revert r321089: "[DAG] Elide overlapping store" (and subsequent fix in r321204)
Jan 2 2018, 6:39 AM

Dec 13 2017

djasper accepted D41195: [ClangFormat] IndentWrappedFunctionNames should be true in the google ObjC style.
Dec 13 2017, 11:51 PM

Dec 11 2017

djasper edited reviewers for D41074: [ClangFormat] ObjCSpaceBeforeProtocolList should be true in the google style, added: thakis; removed: djasper, klimek.
Dec 11 2017, 10:20 AM

Dec 8 2017

djasper added a reviewer for D40909: [clang-format] Reorganize raw string delimiters: klimek.
Dec 8 2017, 2:03 AM
djasper added inline comments to D40909: [clang-format] Reorganize raw string delimiters.
Dec 8 2017, 2:03 AM

Dec 7 2017

djasper added inline comments to D40909: [clang-format] Reorganize raw string delimiters.
Dec 7 2017, 5:25 AM

Nov 30 2017

djasper accepted D40642: clang-format: [JS] do not wrap after async/await..

Looks good.

Nov 30 2017, 2:23 AM

Nov 24 2017

djasper accepted D40424: clang-format: [JS] handle semis in generic types..

Looks good.

Nov 24 2017, 9:03 AM
djasper accepted D40430: clang-format: [JS] do not collapse short classes..

Looks good.

Nov 24 2017, 8:40 AM
djasper added inline comments to D40441: clang-format: [JS] handle `for` as object label..
Nov 24 2017, 8:39 AM
djasper accepted D40436: clang-format: [JS] do not break in ArrayType[]..

Looks good.

Nov 24 2017, 8:38 AM
djasper accepted D40431: clang-format: [JS] do not wrap before yield..

Looks good.

Nov 24 2017, 8:37 AM
djasper accepted D40433: clang-format: [JS] space between ! assert and in..

Looks good.

Nov 24 2017, 8:37 AM
djasper accepted D40411: clang-format: [JS] handle destructuring `of`..

Looks good.

Nov 24 2017, 2:47 AM

Nov 21 2017

djasper edited reviewers for D40288: [clang-format] Add option to group multiple #include blocks when sorting includes, added: krasimir; removed: djasper.
Nov 21 2017, 7:21 AM

Nov 17 2017

djasper accepted D40178: clang-format: [JS] remove trailing lines in arrow functions..

Looks good. There is a chance that some people do not want this in their coding style. But if so, we can add an option later.

Nov 17 2017, 9:49 AM
djasper added a comment to D40178: clang-format: [JS] remove trailing lines in arrow functions..

Is this different for C++ lambdas? I would think that we never should add an empty line before the "}" of a child block.

Nov 17 2017, 8:30 AM

Nov 10 2017

djasper committed rL317901: [clang-format] Handle leading comments in using declaration.
[clang-format] Handle leading comments in using declaration
Nov 10 2017, 9:12 AM
djasper closed D39478: [clang-format] Handle leading comments in using declaration.

Submitted as r317901.

Nov 10 2017, 9:11 AM
djasper accepted D39806: [clang-format] Support python-style comments in text protos.
Nov 10 2017, 12:53 AM

Nov 9 2017

djasper added a comment to D33440: clang-format: better handle statement macros.

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 a braced init list. More info:
http://starship.python.net/crew/mwh/toext/node20.html

Nov 9 2017, 1:15 AM

Nov 7 2017

djasper accepted D39478: [clang-format] Handle leading comments in using declaration.

Looks good. Do you have submit access?

Nov 7 2017, 2:19 AM

Nov 6 2017

djasper closed D39587: [clang-format] Handle unary operator overload with arguments and specifiers.
Nov 6 2017, 4:12 AM
djasper accepted D39587: [clang-format] Handle unary operator overload with arguments and specifiers.
Nov 6 2017, 4:12 AM
djasper added a comment to D39587: [clang-format] Handle unary operator overload with arguments and specifiers.

Submitted as r317473. Thank you!

Nov 6 2017, 4:12 AM
djasper committed rL317473: [clang-format] Handle unary operator overload with arguments and specifiers.
[clang-format] Handle unary operator overload with arguments and specifiers
Nov 6 2017, 4:12 AM

Nov 3 2017

djasper added a comment to D39587: [clang-format] Handle unary operator overload with arguments and specifiers.

Looks good, thank you.

Nov 3 2017, 1:48 AM

Nov 1 2017

djasper accepted D39478: [clang-format] Handle leading comments in using declaration.

Some minor remarks, but generally looks good. Thanks for fixing this!

Nov 1 2017, 12:12 PM

Oct 18 2017

djasper accepted D39024: [clang-format] Sort whole block of using declarations while partially formatting.

LG.

Oct 18 2017, 2:21 PM
djasper committed rL316121: Creating tags/google/stable/2017-10-18 from r315680.
Creating tags/google/stable/2017-10-18 from r315680
Oct 18 2017, 12:40 PM
djasper committed rL316122: Updating branches/google/stable to r315680.
Updating branches/google/stable to r315680
Oct 18 2017, 12:40 PM
djasper committed rL316120: Updating branches/google/stable to r315680.
Updating branches/google/stable to r315680
Oct 18 2017, 12:31 PM
djasper committed rL316119: Creating tags/google/stable/2017-10-18 from r315680.
Creating tags/google/stable/2017-10-18 from r315680
Oct 18 2017, 12:30 PM
djasper committed rL316112: Updating branches/google/stable to r315680.
Updating branches/google/stable to r315680
Oct 18 2017, 12:30 PM
djasper committed rL316111: Creating tags/google/stable/2017-10-18 from r315680.
Creating tags/google/stable/2017-10-18 from r315680
Oct 18 2017, 12:29 PM
djasper committed rL316116: Updating branches/google/stable to r315680.
Updating branches/google/stable to r315680
Oct 18 2017, 12:25 PM
djasper committed rL316115: Creating tags/google/stable/2017-10-18 from r315680.
Creating tags/google/stable/2017-10-18 from r315680
Oct 18 2017, 12:24 PM
djasper committed rL316114: Updating branches/google/stable to r315680.
Updating branches/google/stable to r315680
Oct 18 2017, 12:23 PM
djasper committed rL316113: Creating tags/google/stable/2017-10-18 from r315680.
Creating tags/google/stable/2017-10-18 from r315680
Oct 18 2017, 12:21 PM
djasper added inline comments to D39024: [clang-format] Sort whole block of using declarations while partially formatting.
Oct 18 2017, 11:31 AM

Oct 13 2017

djasper committed rL315680: Revert r314923: "Recommit : Use the basic cost if a GEP is not used as….
Revert r314923: "Recommit : Use the basic cost if a GEP is not used as…
Oct 13 2017, 7:04 AM

Oct 12 2017

djasper accepted D37695: [clang-format] Break non-trailing comments, try 2.

looks good.

Oct 12 2017, 7:11 AM
djasper committed rL315576: Reinstantiate old/bad deduplication logic that was removed in r315279..
Reinstantiate old/bad deduplication logic that was removed in r315279.
Oct 12 2017, 6:25 AM
djasper added a comment to D37979: ClangFormat - Add one space in inline empty function that can be wrapped on a single line. .

I believe that this extra option is not worth its cost in terms of maintainability and (more importantly) discoverability of options. You say that the behavior is documented in your coding style, but it is not (probably for the same reason). There isn't even a single example of this, I think. As such, I don't think we should go forward with adding this option.

Oct 12 2017, 12:35 AM

Oct 11 2017

djasper committed rL315439: Revert r314955: "Remove PendingBody mechanism for function and ObjC method….
Revert r314955: "Remove PendingBody mechanism for function and ObjC method…
Oct 11 2017, 12:48 AM