djasper (Daniel Jasper)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:15 AM (310 w, 1 d)

Recent Activity

Tue, Jun 12

djasper edited reviewers for D48098: clang-format-diff: Switch to python3 by default, support python 2.7, added: krasimir; removed: djasper.
Tue, Jun 12, 11:26 PM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

You are right that this behavior is what the code authors, but also many other people, like to have and so it is what is engrained in clang-format. There are likely about a million things that fall into the same category. Now we might find that the current default is actually wrong and we have changed many defaults in the past. I don't believe that's the case here and there are more opinions on this thread.

Tue, Jun 12, 9:44 AM

Mon, Jun 11

djasper accepted D43015: clang-format: Introduce BreakInheritanceList option.

Looks good. Sorry for the delay.

Mon, Jun 11, 2:21 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

The normal rule for formatting options apply. If you can dig up a public style guide and a project of reasonable size where it is used, we can add an option.

Mon, Jun 11, 2:18 AM

Tue, Jun 5

djasper edited reviewers for D47759: [Format] Do not use a global static value for EOF within ScopedMacroState., added: klimek; removed: djasper.
Tue, Jun 5, 4:25 AM · Restricted Project

May 20 2018

djasper added inline comments to D40988: Clang-format: add finer-grained options for putting all arguments on one line.
May 20 2018, 11:41 PM
djasper edited reviewers for D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name, added: klimek; removed: djasper.
May 20 2018, 11:21 PM

May 8 2018

djasper accepted D46519: [clang-format] Respect BreakBeforeClosingBrace while calculating length.

Generally looks good.

May 8 2018, 3:43 AM
djasper added inline comments to D46519: [clang-format] Respect BreakBeforeClosingBrace while calculating length.
May 8 2018, 2:20 AM

Apr 27 2018

djasper accepted D46143: [clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier.

Looks good, thank you.

Apr 27 2018, 10:17 AM

Apr 20 2018

djasper added inline comments to D40988: Clang-format: add finer-grained options for putting all arguments on one line.
Apr 20 2018, 12:47 AM

Apr 19 2018

djasper accepted D45373: [clang-format] Don't remove empty lines before namespace endings.

Looks good.

Apr 19 2018, 5:14 AM

Apr 12 2018

djasper accepted D45526: [clang-format] Do not break after ObjC category open paren.

Looks good.

Apr 12 2018, 7:58 AM
djasper added a comment to D45526: [clang-format] Do not break after ObjC category open paren.

I understand that, but the test example does not break after the closing paren. It breaks after the subsequent "<".

Apr 12 2018, 6:58 AM
djasper added a comment to D45526: [clang-format] Do not break after ObjC category open paren.

Both patch and comment inside patch say "break after closing paren" and yet that's not what the test or example in the description actually do. Why is that?

Apr 12 2018, 5:36 AM
djasper accepted D45004: [clang-format] Always indent wrapped Objective-C selector names.
Apr 12 2018, 5:33 AM
djasper accepted D45521: [clang-format] Improve ObjC guessing heuristic by supporting all @keywords.

Looks good.

Apr 12 2018, 5:31 AM
djasper accepted D45498: [clang-format] Don't insert space between ObjC class and lightweight generic.

Looks good.

Apr 12 2018, 5:30 AM

Apr 6 2018

djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

Ok, you know the ObjC community much better than I do. If you think that adding the indentation for ObjC irrespective of the option works for most people, I am happy to go with it.

Apr 6 2018, 4:45 AM

Apr 5 2018

djasper added a comment to D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call.

Please read:

Apr 5 2018, 11:35 PM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

I'd go to great lengths to avoid adding new config options and so I don't think this would be a bad trade-off to make.

Apr 5 2018, 9:54 AM
djasper accepted D45185: [clang-format] Support lightweight Objective-C generics.

Looks good, thank you!

Apr 5 2018, 8:15 AM
djasper accepted D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly.
Apr 5 2018, 2:37 AM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

I still really believe that these config options do no make sense and are actively confusing.

Apr 5 2018, 1:55 AM

Apr 3 2018

djasper added inline comments to D45185: [clang-format] Support lightweight Objective-C generics.
Apr 3 2018, 11:44 PM

Apr 2 2018

djasper accepted D45169: [clang-format/ObjC] Do not detect "[]" as ObjC method expression.

Looks good. I like option 2 :).

Apr 2 2018, 10:25 PM
djasper accepted D45168: [clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal.

Looks good.

Apr 2 2018, 10:25 PM

Mar 30 2018

djasper accepted D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames.

Looks good.

Mar 30 2018, 8:03 AM
djasper added inline comments to D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly.
Mar 30 2018, 8:02 AM

Mar 29 2018

djasper added inline comments to D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames.
Mar 29 2018, 8:54 AM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

Well, I disagree. It says: "If you break after the return type of a function declaration or definition, do not indent."

Mar 29 2018, 8:44 AM
djasper added inline comments to D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames.
Mar 29 2018, 5:27 AM
djasper added a reviewer for D45004: [clang-format] Always indent wrapped Objective-C selector names: thakis.
Mar 29 2018, 5:14 AM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

Do we have a public style guide that explicitly says to do this?
That's a requirement for new style options as per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

Mar 29 2018, 5:14 AM

Mar 27 2018

djasper accepted D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal.

Yeah, it's one of these things where neither way would be totally intuitive to everyone.

Mar 27 2018, 7:55 AM
djasper accepted D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines.

Looks good.

Mar 27 2018, 7:54 AM
djasper accepted D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal.

Generally looks good, one minor simplification.

Mar 27 2018, 5:27 AM
djasper added inline comments to D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines.
Mar 27 2018, 2:09 AM

Mar 26 2018

djasper added inline comments to D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines.
Mar 26 2018, 1:33 AM

Mar 23 2018

djasper added inline comments to D44790: [clang-format] Fix ObjC style guesser to also iterate over child lines.
Mar 23 2018, 5:48 AM

Mar 22 2018

djasper accepted D44790: [clang-format] Fix ObjC style guesser to also iterate over child lines.

Looks good, thank you!

Mar 22 2018, 10:29 AM
djasper accepted D42684: clang-format: Allow optimizer to break template declaration..

Some last comments, but basically looks good.

Mar 22 2018, 9:33 AM
djasper accepted D43290: clang-format: tweak formatting of variable initialization blocks.

Generally looks good.

Mar 22 2018, 8:04 AM
djasper committed rL328201: clang-format: Narrow down raw string literal line break exception..
clang-format: Narrow down raw string literal line break exception.
Mar 22 2018, 7:46 AM
djasper committed rC328201: clang-format: Narrow down raw string literal line break exception..
clang-format: Narrow down raw string literal line break exception.
Mar 22 2018, 7:46 AM
djasper committed rL328200: clang-format: Fix SpacesInParentheses with fully qualified names..
clang-format: Fix SpacesInParentheses with fully qualified names.
Mar 22 2018, 7:33 AM
djasper committed rC328200: clang-format: Fix SpacesInParentheses with fully qualified names..
clang-format: Fix SpacesInParentheses with fully qualified names.
Mar 22 2018, 7:33 AM

Mar 21 2018

djasper accepted D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic.

Looks good.

Mar 21 2018, 8:06 AM
djasper accepted D44692: [clang-format] Don't insert space between r_paren and 'new' in ObjC decl.

Looks good.

Mar 21 2018, 8:05 AM
djasper accepted D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set.

Please generally upload diffs with the full file as context so that Phabricator can properly expand the context where necessary.

Mar 21 2018, 5:27 AM

Mar 20 2018

djasper added inline comments to D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic.
Mar 20 2018, 8:37 AM
djasper added inline comments to D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic.
Mar 20 2018, 2:40 AM
djasper added a comment to D44634: [clang-format] Detect Objective-C for #import <Foundation/Foundation.h>.

I'd really like to not parse include/import statements this way. Can you send me examples of headers where we fail to recognize them as ObjC and this matters (happy for you to send me examples offline).

Mar 20 2018, 2:35 AM

Mar 12 2018

djasper accepted D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC.

Ok, looks good.

Mar 12 2018, 8:34 AM
djasper committed rL327255: clang-format: Properly handle implicit string concatenation in text protos.
clang-format: Properly handle implicit string concatenation in text protos
Mar 12 2018, 3:35 AM
djasper committed rC327255: clang-format: Properly handle implicit string concatenation in text protos.
clang-format: Properly handle implicit string concatenation in text protos
Mar 12 2018, 3:35 AM
djasper committed rC327253: Don't re-format raw string literal contents when formatting is disable.
Don't re-format raw string literal contents when formatting is disable
Mar 12 2018, 3:15 AM
djasper committed rL327253: Don't re-format raw string literal contents when formatting is disable.
Don't re-format raw string literal contents when formatting is disable
Mar 12 2018, 3:14 AM

Mar 8 2018

djasper accepted D43906: [clang-format] Improve detection of Objective-C block types.

Nice.. Removed a lot of complexity :). Let's see whether this heuristic is good enough.

Mar 8 2018, 4:01 PM
djasper added inline comments to D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC.
Mar 8 2018, 3:56 PM

Mar 7 2018

djasper added inline comments to D43906: [clang-format] Improve detection of Objective-C block types.
Mar 7 2018, 4:48 AM

Mar 6 2018

djasper added a comment to D43906: [clang-format] Improve detection of Objective-C block types.

Right. So the difference is that for blocks, there is always a "(" after the "(^.....)". That should be easy to parse, no?

Mar 6 2018, 1:32 PM
djasper added inline comments to D43906: [clang-format] Improve detection of Objective-C block types.
Mar 6 2018, 11:34 AM
djasper added inline comments to D43906: [clang-format] Improve detection of Objective-C block types.
Mar 6 2018, 5:37 AM

Mar 5 2018

djasper added a comment to D43906: [clang-format] Improve detection of Objective-C block types.

Would it be enough to only add the block type case? With the macro false positive, there won't be an open paren after the closing paren, right?

Mar 5 2018, 10:54 AM
djasper added inline comments to D43904: [clang-format] Improve detection of ObjC for-in statements.
Mar 5 2018, 10:45 AM
djasper added inline comments to D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC.
Mar 5 2018, 10:40 AM

Mar 2 2018

djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

We have talked about that and none of us agree.

Mar 2 2018, 7:09 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

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 formatter), and its seems our code base is generally formatted like this. I think it is quite more frequent than 50 times in whole LLVM/Clang, but I have no actual numbers; do you have a magic tool to search for such specific "code pattern", or just run clang-format with one option then the next and count the differences ?

Mar 2 2018, 6:48 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

Got two responses so far.

Mar 2 2018, 6:41 AM

Mar 1 2018

djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

Are you sure that you are even addressing an important case? I have done some research on our codebase and this is something that happens incredibly rarely. The reason is that you have to have a very specific combination of line length, where the last parameter does not fit on one line if indented back to align with the open paren while it does fit on multiple lines if indented right of the comma. In all of LLVM/Clang, there seem to be only about 50 cases. How certain are you that people actually care?

Mar 1 2018, 7:12 AM
djasper added a comment to D42684: clang-format: Allow optimizer to break template declaration..

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 template is required because the name is too long).

Mar 1 2018, 7:03 AM

Feb 28 2018

djasper accepted D32525: [clang-format] Add SpaceBeforeColon option.

Ah, I thought it was somehow possible to create:

Feb 28 2018, 10:00 AM
djasper added inline comments to D32525: [clang-format] Add SpaceBeforeColon option.
Feb 28 2018, 8:36 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

New options for this would not be acceptable IMO. Too much cost for too little benefit.

Feb 28 2018, 7:58 AM
djasper added a comment to D42684: clang-format: Allow optimizer to break template declaration..

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 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(
    const bbbbbbbbbbbbbbbbbbb & cccccccccc);

instead of:

template<> void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(
    const bbbbbbbbbbbbbbbbbbb & cccccccccc);

Then when this is fixed the penalty for breaking after the templates part is hardcoded to 10 (prec::Level::Relational), which was not always wrapping as expected (that is definitely subjective, but that is the beauty of penalties...)

Feb 28 2018, 7:52 AM
djasper added a comment to D43015: clang-format: Introduce BreakInheritanceList option.

If both this and https://reviews.llvm.org/D32525 are submitted, then we also need more tests for the combination of the two parameters.

Feb 28 2018, 7:37 AM
djasper added a comment to D32525: [clang-format] Add SpaceBeforeColon option.

I think this generally looks good, but needs a few more tests.

Feb 28 2018, 7:20 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

But you *do* want extra indentation in the case of:

Feb 28 2018, 7:15 AM
djasper added a comment to D42684: clang-format: Allow optimizer to break template declaration..

I think it's possible that this is just a bug/oversight. But I don't fully understand the case where it is not behaving as you expect. Can you give me an example (config setting + code that's not formatted as you expect)?

Feb 28 2018, 7:15 AM
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.

    Unrelated, my point remains that setting BlockKind in TokenAnnotator is bad enough that I wouldn't want to do it for reaping this small benefit. And I can't see how you could easily achieve the same thing without doing that.

Just a question though. I there a reason brace matching (and other parts of TokenAnnotations) are not performed before LineUnwrapping? That would probably allow fixing this issue more cleanly (though I am not sure I would have the time to actually perform this probably significant task)...

Feb 28 2018, 7:08 AM

Feb 27 2018

djasper added inline comments to D43290: clang-format: tweak formatting of variable initialization blocks.
Feb 27 2018, 8:49 AM
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)

I don't understand this. Which style do you actually care about? With Cpp11BracedListStyle=true or =false?

I use the Cpp11BracedListStyle=false style.
However, I think the current behavior is wrong also when Cpp11BracedListStyle=true. Here the behavior in this case:

Before :

const std::unordered_map<std::string, int> Something::MyHashTable =
    {{ "aaaaaaaaaaaaaaaaaaaaa", 0 },
     { "bbbbbbbbbbbbbbbbbbbbb", 1 },
     { "ccccccccccccccccccccc", 2 }};

After :

const std::unordered_set<std::string> Something::MyUnorderedSet = {
    { "aaaaaaaaaaaaaaaaaaaaa", 0 },
    { "bbbbbbbbbbbbbbbbbbbbb", 1 },
    { "ccccccccccccccccccccc", 2 }};
Feb 27 2018, 6:30 AM

Feb 23 2018

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

Submitted as r326023.

Feb 23 2018, 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
Feb 23 2018, 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
Feb 23 2018, 10:56 PM
djasper created D43673: Make module use diagnostics refer to the top-level module.
Feb 23 2018, 5:25 AM

Feb 21 2018

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

Thank you for doing these follow up changes!

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

Feb 20 2018

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)

Feb 20 2018, 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?

Feb 20 2018, 2:29 AM

Feb 19 2018

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

Looks good.

Feb 19 2018, 4:10 AM

Feb 15 2018

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

Feb 15 2018, 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.

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

Looks good.

Feb 15 2018, 5:34 AM
djasper edited reviewers for D43312: [clang-format] fix handling of consecutive unary operators, added: krasimir; removed: djasper.
Feb 15 2018, 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?

Feb 15 2018, 4:51 AM