klimek (Manuel Klimek)Administrator
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 7 2012, 2:55 PM (318 w, 3 d)
Roles
Administrator

Recent Activity

Wed, Aug 1

klimek added a comment to D45719: [clang-Format] Fix indentation of member call after block.

Sorry that I lost track of that, but feel free to ping once / week - unfortunately the patch doesn't apply cleanly, can you rebase?

Wed, Aug 1, 8:43 AM
klimek added a comment to D37813: clang-format: better handle namespace macros.

The problem here is that we have different opinions on how the formatting on namespace macros should behave in the first place- I think they should behave like namespaces, you want them to be formatted differently.
Given that you want full control over the formatting of those macros, and them not actually be formatted exactly like namespaces or classes, I think we need a more generic mechanism for you to express that.

Wed, Aug 1, 3:15 AM
klimek added a comment to D50078: clang-format: support aligned nested conditionals formatting.

Yea, if we go down this route I'd go with this by default:

some ? thing :
else ? otherthing :
unrelated ? but :
    finally;
Wed, Aug 1, 3:03 AM

Thu, Jul 26

klimek added a comment to D49840: [AST] Add MatchFinder::matchSubtree.

Usually we use match(anyOf(node), hasDescendant(node)). Or did I misunderstand what you want?

Thu, Jul 26, 3:21 AM

Tue, Jul 24

klimek accepted D49724: [VFS] Cleanups to VFS interfaces..

lg

Tue, Jul 24, 4:45 AM

Jul 13 2018

klimek added inline comments to D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style).
Jul 13 2018, 2:28 AM

Jul 3 2018

klimek added inline comments to D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style).
Jul 3 2018, 2:28 AM
klimek added inline comments to D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style).
Jul 3 2018, 12:09 AM

Jul 2 2018

klimek added a comment to D28462: clang-format: Add new style option AlignConsecutiveMacros.

@klimek having gotten that out of the way, I do occasionally drink too much and have sudden urges to re-implement things from scratch. Close it if you need to, since I can't commit to anything, but.... it it happens to be still open on one of those nights, who knows, maybe I'll end up doing it :)

Jul 2 2018, 12:45 AM · Restricted Project
klimek added inline comments to D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly.
Jul 2 2018, 12:44 AM · Restricted Project
klimek accepted D48159: [clangd] Implement hover for "auto" and "decltype".
Jul 2 2018, 12:22 AM

Jun 29 2018

klimek added a reviewer for D48159: [clangd] Implement hover for "auto" and "decltype": rsmith.
Jun 29 2018, 2:47 AM
klimek added a comment to D28462: clang-format: Add new style option AlignConsecutiveMacros.

@klimek fair point. To be honest, I've pretty much lost interest / momentum on this feature, I very much doubt I will ever go back and re-implement from scratch as you suggest.
Not meaning to sound rude, I just don't want to waste anyone's time who is waiting for this (seems there are a few).

Jun 29 2018, 2:38 AM · Restricted Project

Jun 28 2018

klimek added inline comments to D48159: [clangd] Implement hover for "auto" and "decltype".
Jun 28 2018, 1:18 AM

Jun 27 2018

klimek added a comment to D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style).
Hello,

after my last modification (require by previous comment), I do not see any feedback or validation for this patch.
Is their something special to do in order to go forward on this patch?

Lambda are more an more used in modern C++, and it's very annoying to not have a way to format them in allman style.

Jun 27 2018, 12:35 AM

Jun 26 2018

klimek accepted D45719: [clang-Format] Fix indentation of member call after block.

LG.

Jun 26 2018, 4:56 AM
klimek added inline comments to D45719: [clang-Format] Fix indentation of member call after block.
Jun 26 2018, 1:12 AM

Jun 25 2018

klimek added inline comments to D45719: [clang-Format] Fix indentation of member call after block.
Jun 25 2018, 8:54 AM
klimek added inline comments to D45719: [clang-Format] Fix indentation of member call after block.
Jun 25 2018, 4:30 AM

Jun 21 2018

klimek added inline comments to D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab.
Jun 21 2018, 6:56 AM · Restricted Project

Jun 18 2018

klimek accepted D48242: [ASTMatchers] Add support for matching the type of a friend decl..

lg, thx

Jun 18 2018, 2:12 AM · Restricted Project
klimek accepted D48269: [ASTMatchers] Don't assert-fail in specifiesTypeLoc()..

lg, thanks!

Jun 18 2018, 1:59 AM · Restricted Project

Jun 17 2018

klimek added inline comments to D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab.
Jun 17 2018, 11:11 PM · Restricted Project

Jun 15 2018

klimek added inline comments to D48242: [ASTMatchers] Add support for matching the type of a friend decl..
Jun 15 2018, 5:02 PM · Restricted Project
klimek added a comment to D48230: [PredicateInfo] Order instructions in different BBs by DFSNumIn..

Florian: did you manually add Danny on this (the account is disabled and I'm trying to figure out where / how folks are added).

Jun 15 2018, 4:52 PM

Jun 14 2018

klimek accepted D47759: [Format] Do not use a global static value for EOF within ScopedMacroState..

LG.

Jun 14 2018, 9:04 PM · Restricted Project

Jun 12 2018

klimek accepted D46024: [clang-format] Add SpaceBeforeCpp11BracedList option..

FWIW, please note that this space-before-brace style is not specific to WebKit; CppCoreGuidelines exhibits it as well:
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax

This and WebKit's style seem like compelling arguments to support this option.

klimek, djasper: Do you have any objections against landing this?

Jun 12 2018, 10:27 AM

May 30 2018

klimek accepted D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name.

LG

May 30 2018, 2:17 AM

May 9 2018

klimek committed rCTE331875: Do not warn on unused parameters for functions with empty bodies..
Do not warn on unused parameters for functions with empty bodies.
May 9 2018, 6:23 AM
klimek committed rL331875: Do not warn on unused parameters for functions with empty bodies..
Do not warn on unused parameters for functions with empty bodies.
May 9 2018, 6:23 AM

May 2 2018

klimek added a comment to D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy.

I believe this was accepted by rnk - are you waiting for specific further feedback?

May 2 2018, 12:54 AM

Apr 26 2018

klimek added a comment to D46024: [clang-format] Add SpaceBeforeCpp11BracedList option..

Is this written down somewhere? https://webkit.org/code-style-guidelines/ doesn't seem to mention it.

Apr 26 2018, 6:01 AM
klimek added inline comments to D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style).
Apr 26 2018, 6:01 AM

Apr 25 2018

klimek accepted D46062: [clang-format] Start formatting cpp code in raw strings in google style.

LG

Apr 25 2018, 7:42 AM
klimek added inline comments to D45719: [clang-Format] Fix indentation of member call after block.
Apr 25 2018, 3:27 AM

Apr 23 2018

klimek committed rCTE330580: Fix tests after changes to clang-format in r330573..
Fix tests after changes to clang-format in r330573.
Apr 23 2018, 4:51 AM
klimek committed rL330580: Fix tests after changes to clang-format in r330573..
Fix tests after changes to clang-format in r330573.
Apr 23 2018, 4:51 AM
klimek committed rL330573: Format closing braces when reformatting the line containing the opening brace..
Format closing braces when reformatting the line containing the opening brace.
Apr 23 2018, 2:37 AM
klimek committed rC330573: Format closing braces when reformatting the line containing the opening brace..
Format closing braces when reformatting the line containing the opening brace.
Apr 23 2018, 2:37 AM
klimek closed D45726: Format closing braces when reformatting the line containing theopening brace..
Apr 23 2018, 2:37 AM

Apr 20 2018

klimek added a comment to D45726: Format closing braces when reformatting the line containing theopening brace..

Another point: for the example in the summary about bailing-out early, is there a test for this already? If not, we should add one.

Apr 20 2018, 3:04 AM
klimek updated the diff for D45726: Format closing braces when reformatting the line containing theopening brace..

Address comments.

Apr 20 2018, 3:04 AM
klimek added inline comments to D28462: clang-format: Add new style option AlignConsecutiveMacros.
Apr 20 2018, 1:05 AM · Restricted Project

Apr 17 2018

klimek created D45726: Format closing braces when reformatting the line containing theopening brace..
Apr 17 2018, 9:10 AM
klimek accepted D38615: [libclang] Only mark CXCursors for explicit attributes with a type.

lg

Apr 17 2018, 2:31 AM

Apr 11 2018

klimek committed rL329816: Fix bugs around handling C++11 attributes..
Fix bugs around handling C++11 attributes.
Apr 11 2018, 7:55 AM
klimek committed rC329816: Fix bugs around handling C++11 attributes..
Fix bugs around handling C++11 attributes.
Apr 11 2018, 7:55 AM

Apr 9 2018

klimek accepted D43969: Improve completion experience for headers.

lg

Apr 9 2018, 6:29 AM

Jan 16 2018

klimek added a comment to D42036: [clang-format] Keep comments aligned to macros.

Just from a formatting point of view, why not:

//.   Comment
#.    define X
Jan 16 2018, 1:16 AM

Jan 5 2018

klimek added inline comments to D41487: [clang-format] Adds a FormatStyleSet.
Jan 5 2018, 7:07 AM

Dec 21 2017

klimek added inline comments to D41487: [clang-format] Adds a FormatStyleSet.
Dec 21 2017, 7:06 AM

Dec 18 2017

klimek added a comment to D37813: clang-format: better handle namespace macros.
In D37813#958116, @Typz wrote:

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 want to only allow a small subset of actual macros, but the general idea would be the same: The UnwrappedLineParser would parse the macro at the expansion location A(X) into an unwrapped line, and then parse the expansion into a child line, with the tokens tha tare not in the argument of the call being marked as fixed (parent child might also be better inverted).

(As a side-note, I want to stress out that we would actually need a 'reversible' description to support the namespace case, to allow generating the end-of-namespace comment)

Dec 18 2017, 4:23 AM
klimek added a comment to D37813: clang-format: better handle namespace macros.

I've talked with Daniel and we both believe this patch is not the right way to go forward for clang-format. It is adding configuration mechanisms that we need to maintain going forward, without addressing a general problem; specifically for how to handle macros, I believe there is a clear better way to do it that is also fundamentally more useful than adding extra knobs for every problem we encounter. Finally, these changes do not address a fundamental bug, and workarounds exist, so there is no large benefit to do this now in a non-principled way rather than wait for a more principled fix. Overall, I do understand that this is a trade-off, but I hope you understand that we sometimes have to make decisions.

Dec 18 2017, 1:40 AM

Dec 13 2017

klimek accepted D41130: git-clang-format: cleanup: Use run() when possible..

lg

Dec 13 2017, 1:05 AM
klimek accepted D41147: git-clang-format: Add new --staged option..

lg

Dec 13 2017, 1:03 AM
klimek accepted D41145: git-clang-format: refactor to support upcoming --staged flag.

lg

Dec 13 2017, 1:02 AM

Dec 11 2017

klimek added inline comments to D40485: [clangd] Introduced a Context that stores implicit data.
Dec 11 2017, 7:32 AM

Dec 8 2017

klimek added inline comments to D40909: [clang-format] Reorganize raw string delimiters.
Dec 8 2017, 2:33 AM

Dec 6 2017

klimek added inline comments to D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions.
Dec 6 2017, 7:20 AM
klimek added a comment to D37813: clang-format: better handle namespace macros.
In D37813#945125, @Typz wrote:

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 class (and thus make changes to clang-format configuration). Here we are not defining what the macro does, but how clang-format should indent it : in most case I don't think the indentation format should actually depend on the way it is implemented...

Dec 6 2017, 2:31 AM

Dec 4 2017

klimek committed rC319642: Fix bug where we wouldn't break columns over the limit..
Fix bug where we wouldn't break columns over the limit.
Dec 4 2017, 12:54 AM
klimek committed rL319642: Fix bug where we wouldn't break columns over the limit..
Fix bug where we wouldn't break columns over the limit.
Dec 4 2017, 12:53 AM

Dec 1 2017

klimek added a comment to D37813: clang-format: better handle namespace macros.
In D37813#942137, @Typz wrote:

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 totally related; it would seem very strange to have an option to specify that TESTSUITE() macros are parsed as namespace, then another option to indicate that namespace declared by this macros are actually closed with another macro call...

Dec 1 2017, 7:55 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.
In D33589#942128, @Typz wrote:

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 overflow a bit, but it gets unindented during "alingment" stage, which kind of 'breaks' the decision that was made earlier on *not* to break the comment...

Dec 1 2017, 7:47 AM
klimek added a comment to D37813: clang-format: better handle namespace macros.
In D37813#941987, @Typz wrote:

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 that is really not trivial, I would prefer to move forward with these "simpler" patch, and change the whole macro configurations part in later commits...

Dec 1 2017, 6:35 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.
In D33589#941979, @Typz wrote:

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 will often get closer to the "extra" limit.

That said, I tried with your latest change ("Restructure how we break tokens", sha1:64d42a2fb85ece5987111ffb908c6bc7f7431dd4). and it's working about fine now. For the most part it seems to wrap when I would expect it, great work!
I have seen 2 "issues" though:

  • Often I see that the last word before reflowing is not wrapped (eventhough it overlaps the line length); I did not count penalties so I cannot confirm this is really an issue or just a borderline scenario.
  • Alignment seems better than before, but since there is no penalty for breaking alignment it will always try to unindent to compensate for overflowing characters...

    Seeing this, I guess this patch does not make much sense anymore, I'll see if I make some improvements for these two issues, in separate patches.
Dec 1 2017, 5:54 AM
klimek committed rC319541: Better trade-off for excess characters vs. staying within the column limits..
Better trade-off for excess characters vs. staying within the column limits.
Dec 1 2017, 5:29 AM
klimek committed rL319541: Better trade-off for excess characters vs. staying within the column limits..
Better trade-off for excess characters vs. staying within the column limits.
Dec 1 2017, 5:28 AM
klimek closed D40605: Better trade-off for excess characters vs. staying within the column limits. by committing rL319541: Better trade-off for excess characters vs. staying within the column limits..
Dec 1 2017, 5:28 AM
klimek updated the diff for D40605: Better trade-off for excess characters vs. staying within the column limits..

Add test.

Dec 1 2017, 3:04 AM

Nov 30 2017

klimek updated the diff for D40605: Better trade-off for excess characters vs. staying within the column limits..

Fix incorrect return value leading to unnecessary computation.

Nov 30 2017, 7:52 AM
klimek added inline comments to D40605: Better trade-off for excess characters vs. staying within the column limits..
Nov 30 2017, 6:16 AM

Nov 29 2017

klimek created D40605: Better trade-off for excess characters vs. staying within the column limits..
Nov 29 2017, 7:53 AM
klimek committed rL319318: Fix 'control reaches end of non-void' warning by using llvm_unreachable..
Fix 'control reaches end of non-void' warning by using llvm_unreachable.
Nov 29 2017, 7:09 AM
klimek committed rC319318: Fix 'control reaches end of non-void' warning by using llvm_unreachable..
Fix 'control reaches end of non-void' warning by using llvm_unreachable.
Nov 29 2017, 7:09 AM
klimek committed rC319314: Restructure how we break tokens..
Restructure how we break tokens.
Nov 29 2017, 6:30 AM
klimek committed rL319314: Restructure how we break tokens..
Restructure how we break tokens.
Nov 29 2017, 6:30 AM
klimek closed D40310: Restructure how we break tokens. by committing rL319314: Restructure how we break tokens..
Nov 29 2017, 6:30 AM
klimek added inline comments to D40310: Restructure how we break tokens..
Nov 29 2017, 4:32 AM
klimek added inline comments to D40310: Restructure how we break tokens..
Nov 29 2017, 3:09 AM
klimek updated the diff for D40310: Restructure how we break tokens..

Address review comments.

Nov 29 2017, 3:09 AM

Nov 28 2017

klimek added inline comments to D40310: Restructure how we break tokens..
Nov 28 2017, 8:30 AM
klimek updated the diff for D40310: Restructure how we break tokens..

Address review comments.

Nov 28 2017, 8:28 AM
klimek accepted D40072: [libclang] Support querying whether a declaration is invalid.

LG

Nov 28 2017, 1:44 AM

Nov 27 2017

klimek accepted D40494: [clang] Set up .arcconfig to point to new Diffusion C repository.

LG. Thx!

Nov 27 2017, 8:16 AM
klimek added a comment to D40310: Restructure how we break tokens..

Restructured to make the invariants clearer based on a chat with Krasimir.

Nov 27 2017, 7:57 AM
klimek updated the diff for D40310: Restructure how we break tokens..

Restructure based on code review feedback.

Nov 27 2017, 7:56 AM
klimek abandoned D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..
Nov 27 2017, 7:55 AM
klimek added inline comments to D40310: Restructure how we break tokens..
Nov 27 2017, 2:55 AM
klimek accepted D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..

Self-accepting and closing, as the underlying change has landed, and this diff is incorrect now.
I also have an idea to solve the problem Typz has brought up.

Nov 27 2017, 2:27 AM

Nov 23 2017

klimek added inline comments to D40310: Restructure how we break tokens..
Nov 23 2017, 8:34 AM
klimek updated the diff for D40310: Restructure how we break tokens..

Pull out getRemainingLength.

Nov 23 2017, 8:32 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.
In D33589#933746, @Typz wrote:

with this setting, a "non wrapped" comment will actually be reflown to ColumnLimit+10...

Nov 23 2017, 7:05 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.
In D33589#933746, @Typz wrote:

@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 fixes it - it will only ever reflow below the column limit, so it'll also lead to states for long lines where reflowing and leaving chars over the line limit might be the overall best choice (unless I'm missing something)?

Definitely, this patch may not find the 'optimal' solution. What I mean is that we reduce the PenaltyExcessCharacter value to allow "occasionally" breaking the limit: instead of a hard limit, we want to allow lines to sometimes break the limit, but definitely not *all* the time. Both patch work fine when the code is "correct", i.e. there is indeed only a few lines which break the limit.

But the local decision approach behaves really wrong IMHO when the code is formatted beyond the column: it can very easily reformat in such a way that the comment is reflown to what looks like a longer column limit. I currently have a ratio of 10 between PenaltyBreakComment and PenaltyExcessCharacter (which empirically seemed to give a decent compromise, and match how our code is formatted; I need to try more with your patch, to see if we can get better values...): with this setting, a "non wrapped" comment will actually be reflown to ColumnLimit+10...

Nov 23 2017, 7:03 AM
klimek added a comment to D37813: clang-format: better handle namespace macros.
In D37813#930065, @Typz wrote:

ping?

Nov 23 2017, 6:48 AM
klimek added inline comments to D40310: Restructure how we break tokens..
Nov 23 2017, 6:32 AM
klimek updated the diff for D40310: Restructure how we break tokens..

Address review comments.

Nov 23 2017, 6:32 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.
In D33589#931723, @Typz 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 D40068.

Can you provide more info on these cases? Are they all added to the tests of D40068?

It may be simpler (though not to my eyes, I am not knowledgeable enough to really understand how you go this fixed...), and works fine for "almost correct" comments: e.g. when there are indeed just a few extra characters overall. But it still procudes strange result when each line of the (long) comment is too long, but not enough to trigger a line-wrap by itself.

Nov 23 2017, 3:34 AM
klimek added a comment to D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..
In D40068#931679, @Typz wrote:

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 each line (e.g. is it better to wrap this line or overflow a bit), so we can get a comment where each line overflows by a few characters, even if the total is worse... For exemple, say we have a 100 lines of comment, with 9 characters overflow on each line, and an excess character penalty of 30 : with this patch nothing will be re-wrapped (9*30 = 270 is less the the 300 penalty for wrapping); but the total penatly would be 900....

(btw, it seems this got merged, but the ticket does not reflect it)

Nov 23 2017, 2:33 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.
In D33589#931802, @Typz wrote:

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, ///< This is a very long
        ///< comment
   Bar, ///< This is shorter  
   BarBar, ///< This is shorter
} Stuff;

when I would expect:

enum {
   Foo,    ///< This is a very
           ///< long comment
   Bar,    ///< This is shorter  
   BarBar, ///< This is shorter
} Stuff;

I see there is no penalty for breaking alignment, which may be accounted for when compressing the whitespace 'before' the comment... Or maybe simply the breaking should be smarter, to try to keep the alignment; but I am not sure it can be done without another kind of 'global' optimization of the comment reflow... Any idea/hint?

Nov 23 2017, 2:30 AM