klimek (Manuel Klimek)Administrator
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 7 2012, 2:55 PM (289 w, 4 h)
Roles
Administrator

Recent Activity

Tue, Jan 16

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
Tue, Jan 16, 1:16 AM

Fri, Jan 5

klimek added inline comments to D41487: [clang-format] Adds a FormatStyleSet.
Fri, Jan 5, 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
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.

Since that version has landed already, not sure how to improve on this. I could probably rewrite my patch on master, but it seems a bit redundant. As a simpler fix, I could imagine adding a "total" overflow counter, to allow detecting the situation; but when this is detected (e.g. on subsequent lines) we would need to "backtrack" and revisit the initial decision...

Nov 23 2017, 2:27 AM
klimek added inline comments to D40060: [clangd] Fuzzy match scorer.
Nov 23 2017, 1:43 AM

Nov 21 2017

klimek created D40310: Restructure how we break tokens..
Nov 21 2017, 8:01 AM

Nov 17 2017

klimek updated the diff for D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..

Just exporting

Nov 17 2017, 9:20 AM
klimek accepted D40120: [clang-format] Add text proto filename detection.

LG

Nov 17 2017, 6:36 AM
klimek committed rL318515: Implement more accurate penalty & trade-offs while breaking protruding tokens..
Implement more accurate penalty & trade-offs while breaking protruding tokens.
Nov 17 2017, 3:18 AM
klimek added inline comments to D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..
Nov 17 2017, 3:09 AM
klimek updated the diff for D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..

Address review comments.

Nov 17 2017, 3:09 AM

Nov 16 2017

klimek added inline comments to D39842: Allow to store precompiled preambles in memory..
Nov 16 2017, 5:40 AM
klimek added a comment to D40120: [clang-format] Add text proto filename detection.

Should that be configured? METADATA seems pretty domain specific ;)

Nov 16 2017, 4:49 AM
klimek added inline comments to D39842: Allow to store precompiled preambles in memory..
Nov 16 2017, 4:49 AM
klimek added inline comments to D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..
Nov 16 2017, 2:08 AM
klimek updated the diff for D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..

Address review comments.

Nov 16 2017, 2:08 AM
klimek accepted D39842: Allow to store precompiled preambles in memory..

LG

Nov 16 2017, 2:00 AM

Nov 15 2017

klimek added inline comments to D39842: Allow to store precompiled preambles in memory..
Nov 15 2017, 3:29 AM
klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.

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.

Nov 15 2017, 2:51 AM
klimek created D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens..
Nov 15 2017, 2:15 AM

Nov 14 2017

klimek added a comment to D33589: clang-format: consider not splitting tokens in optimization.

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 handles all tests in this patch correctly, but is fundamentally limiting the global optimizations we can do. Specifically, we would not correctly reflow this:

//       |< limit
// foo bar
// baz
// x

to

// foo
// bar
// baz x

when the excess character limit is low.

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

ping ?

Nov 14 2017, 2:17 AM
klimek committed rL318141: Refactor ContinuationIndenter's breakProtrudingToken logic..
Refactor ContinuationIndenter's breakProtrudingToken logic.
Nov 14 2017, 1:20 AM
klimek closed D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly more orthogonal pieces. by committing rL318141: Refactor ContinuationIndenter's breakProtrudingToken logic..
Nov 14 2017, 1:20 AM
klimek added a comment to D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly more orthogonal pieces..

Maybe we should further refactor getRawStringStyle into llvm::Optional<std::pair<style, delimiter>> getRawStringStyleAndDelimiter and that would nicely take care of the duplicated effort?

Nov 14 2017, 1:12 AM

Nov 13 2017

klimek added a comment to D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set.
In D32478#920345, @Typz wrote:

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 consistency at entreprise scale, thus as a general rule we should not change the established rules in an organizatoin.
There is history in rules, we have code which already uses these rules, so for consistency we must stick to the old rules (even if we would not necessarily choose the same rules if we were to start from scratch).

Nov 13 2017, 5:11 AM

Nov 10 2017

klimek added inline comments to D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly more orthogonal pieces..
Nov 10 2017, 7:36 AM
klimek updated the diff for D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly more orthogonal pieces..
  • Add test.
Nov 10 2017, 7:36 AM
klimek added inline comments to D39842: Allow to store precompiled preambles in memory..
Nov 10 2017, 7:30 AM
klimek accepted D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS.

LG.

Nov 10 2017, 7:02 AM
klimek created D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly more orthogonal pieces..
Nov 10 2017, 6:57 AM

Nov 9 2017

klimek added a comment to D39836: [clangd] Drop impossible completions (unavailable or inaccessible).

I personally think they're useful to have anyway, and they don't add much noise when they're at the end of completions list.
I sometimes want to complete an item I know is there and then fix accessibility. Do you think they add too much noise?

For me the noise definitely outweighs the value, e.g. when I'm trying to see all the available functions. There are costs beyond noise:

  • they bulk up tests and debugging output
  • they complicate the scoring scheme - we *never* want them to appear above a legal completion, so a 1-dimensional score needs special handling as here.
  • compute: we spend time scoring them, computing their strings, writing them over the wire, and then the client has to process them
Nov 9 2017, 3:29 AM
klimek added a comment to D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set.
In D32478#920275, @Typz wrote:

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 precise enough, and do not document the behavior for this case.

I have problems understanding the rules from the discussion (as well as the code / test tbh), is there a really concise way to give me the core of the idea?

The case I am trying to address is to really keep the _operands_ aligned after an assignment or return, in case line is wrapped *before* the operator.

int a = b
      + c;
return a
     + b;

while the current behavior with Style.AlignOperands = true; BreakBeforeBinaryOperators = true is to align the wrapped operator with the previous line's operand:

int a = b
        + c;
return a
       + b;

In the discussion, it appeared that this behavior is not a error (with respect to the name), but an expected behavior for most coding rules: hence the introduction of a third AlignOperands mode ("AlignAfterOperator"), to handle this new case.

From there the code actually got a bit more complex to support various corner cases (e.g. operators with different number of characters, wrapperd first line, do not unindent more than the enclosing brace...)

Nov 9 2017, 2:55 AM
klimek accepted D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found..

lg

Nov 9 2017, 2:11 AM
klimek added a comment to D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set.
In D32478#806757, @Typz wrote:

I renamed the option to AlignAfterOperator, is it acceptable now?
(I also thought of UnindentOperator, but I think it is better to keep the notion of alignment)

Note that this style is used in multiple open-source projects: Skia, parts of QtCreator code base (e.g. in multiple plugins: clang, cpptools, android...), the OpenEXR library...

Nov 9 2017, 1:00 AM

Nov 8 2017

klimek added inline comments to D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found..
Nov 8 2017, 6:21 AM

Nov 7 2017

klimek accepted D36827: Changed createTemporaryFile without FD to actually create a file..

Looks good. Sorry for the delay in review.

Nov 7 2017, 2:42 AM
klimek added a comment to D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy.

Sorry for jumping in late, but I'm somewhat questioning whether an extra function in the API is worth the cost here:
If you already have a printing policy, getting the type name will be 1 line instead of 2; having an extra function in a large API seems not worth it at a first glance.

Nov 7 2017, 1:39 AM
klimek added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.

Weekly ping. This week is C++ committee, so it's again going to be hard to get Richards attention :( It's an unfortunate time of year to land major API-touching changes.

Nov 7 2017, 1:05 AM

Oct 27 2017

klimek added inline comments to D35943: [clang-format] Format raw string literals.
Oct 27 2017, 2:01 AM

Oct 26 2017

klimek accepted D35943: [clang-format] Format raw string literals.

Needs a bit more comments, otherwise LG.

Oct 26 2017, 10:01 AM
klimek accepted D34272: [Tooling] A new framework for executing clang frontend actions..

lg

Oct 26 2017, 1:55 AM
klimek added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.

Just FYI, I talked with Richard and he'll not get to it before next week. Hope that's fine.

Oct 26 2017, 12:30 AM

Oct 25 2017

klimek added inline comments to D34272: [Tooling] A new framework for executing clang frontend actions..
Oct 25 2017, 8:09 AM

Oct 23 2017

klimek added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.

Ok, I think this is starting to look really good. Now we should try to grab Richard's attention to make sure it's fine to submit with the current set of FIXMEs.

Oct 23 2017, 6:37 PM
klimek added inline comments to D39180: JSON library (just a prototype at this point)..
Oct 23 2017, 10:22 AM

Oct 17 2017

klimek added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.

Whee, thanks for driving this, much appreciated :D

Oct 17 2017, 3:18 PM
klimek added inline comments to D34272: [Tooling] A new framework for executing clang frontend actions..
Oct 17 2017, 9:10 AM

Oct 12 2017

klimek added a comment to D38818: Template Instantiation Observer + a few other templight-related changes.

Please make sure patches get sent to cfe-commits in addition to the reviewers, as per https://llvm.org/docs/Phabricator.html

Oct 12 2017, 2:16 PM
klimek added a comment to D37979: ClangFormat - Add one space in inline empty function that can be wrapped on a single line. .

I also see lots of code that uses {}? Thus, it seems like the code base is not very strict, and going either way enforced with clang-format will be nicer?
https://dxr.mozilla.org/mozilla-central/search?q=%22+%7B%7D&redirect=true

Indeed, this is why we are working on enforcing that.
This patch is one of the last change to match exactly our coding style. This is why Andi wrote it.

If you could accept it, it would be great. If you won't, please let us know to see what we can do on our side. Thanks!

Oct 12 2017, 7:51 AM

Oct 11 2017

klimek added a comment to D37979: ClangFormat - Add one space in inline empty function that can be wrapped on a single line. .

Also doing a grep in our online repository:
https://dxr.mozilla.org/mozilla-central/search?q=%22%7B+%7D&redirect=false

You can see that the incidence of this behaviour is huge. I know the grep produces also noise but still the occurrence is this scenario is huge.

Oct 11 2017, 4:14 PM