Page MenuHomePhabricator

berenm (Beren Minor)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 27 2015, 11:40 AM (232 w, 5 d)

Recent Activity

Dec 8 2017

berenm added a comment to D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming.

Yes, please could you commit it for me? Thanks!

Dec 8 2017, 10:25 AM
berenm updated the diff for D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming.

Rebase patch on upstream HEAD

Dec 8 2017, 10:24 AM

Dec 7 2017

berenm added a comment to D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming.

Sorry for the delay. I missed this revision somehow. Please add cfe-commits to the subscribers list so that others can chime in.

Dec 7 2017, 12:16 PM
berenm updated the diff for D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming.

Factor !Type.isNull() && Type.isConstQualified() condition

Dec 7 2017, 12:14 PM

Oct 27 2017

berenm created D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming.
Oct 27 2017, 12:31 AM

Mar 14 2017

berenm added a comment to D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style.

I understand your use case, but this patch makes the check's behavior more confusing: having both "any case" and "ignore case" with subtle differences in behavior seems very misleading. The problem seems to be coming from the usage of CT_AnyCase to denote uninitialized style. Should we just remove NamingStyle::isSet and use llvm::Optional<NamingStyle> instead of NamingStyle where appropriate?

Mar 14 2017, 9:52 AM

Jan 9 2017

berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

There's https://reviews.llvm.org/D27651 that will conflict with this one.

Jan 9 2017, 1:19 PM · Restricted Project
berenm accepted D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s to the right.

Awesome!

Jan 9 2017, 1:13 PM
berenm added a comment to D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s to the right.

Yes you are right,

Jan 9 2017, 9:39 AM
berenm added a comment to D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s to the right.

I'm trying to think of a scenario where *, && or & before tokens to be aligned would not indicate pointers or references, but as the alignment is only done for now on declarations and assignments, I can't find one.

Jan 9 2017, 6:50 AM

Sep 23 2016

berenm removed a child revision for D21279: Fix some issues in clang-format's AlignConsecutive modes: D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field.
Sep 23 2016, 6:25 AM · Restricted Project
berenm removed a parent revision for D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field: D21279: Fix some issues in clang-format's AlignConsecutive modes.
Sep 23 2016, 6:25 AM
berenm abandoned D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field.
Sep 23 2016, 6:25 AM
berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

Are we talking completely past each other? I specifically think we should *NOT* combine NestingLevel and IndentLevel into one value. Not in ScopeLevel() and not anywhere else.

Sep 23 2016, 6:24 AM · Restricted Project
berenm added a reviewer for D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field: djasper.
Sep 23 2016, 5:54 AM
berenm added a parent revision for D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field: D21279: Fix some issues in clang-format's AlignConsecutive modes.
Sep 23 2016, 5:54 AM
berenm added a comment to D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field.

This patch might help clarifying the NestingLevel usage and can simplify the alignment computations in https://reviews.llvm.org/D21279

Sep 23 2016, 5:53 AM
berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

I had a little bit of look into the NestingLevel field. I understand that it only indicates the nesting level of the token inside the current AnnotatedLine, which could very well be the same as the nesting level of another token in the previous or next AnnotatedLine. Right now, it doesn't include the information on the nested level of the AnnotatedLine itself (this is in the Level field of the line itself).

Sep 23 2016, 5:45 AM · Restricted Project
berenm retitled D24859: [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field from to [Format] Include AnnotatedLine.Level in FormatToken.NestingLevel field.
Sep 23 2016, 5:43 AM

Aug 27 2016

jsmouret awarded D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk a Like token.
Aug 27 2016, 8:24 AM

Jun 21 2016

berenm accepted D21279: Fix some issues in clang-format's AlignConsecutive modes.
Jun 21 2016, 2:18 AM · Restricted Project
berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.
  1. friend functions: I don't really understand why the current behavior is what it is, but I think it's reasonable to argue that it actually improves readability by drawing attention to the fact these are friend functions, which ought to be quite rare in most code

Actually it looks like it works now... I'm not sure what I did when I had this misalignment. It would have been fine by me anyway.

Jun 21 2016, 2:18 AM · Restricted Project

Jun 20 2016

berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

Thanks! The operators are now correctly handled.

Jun 20 2016, 4:22 AM · Restricted Project

Jun 14 2016

berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

It looks like it doesn't like the operator[] either:

Jun 14 2016, 11:45 PM · Restricted Project

Jun 13 2016

berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

There's still cases where the nesting level is still not correctly handled: when using unbraced conditionals / loops. For example (sorry, silly example):

Jun 13 2016, 12:08 PM · Restricted Project
berenm added a comment to D21279: Fix some issues in clang-format's AlignConsecutive modes.

I think it's better to update the diff with full context (git diff -U999999) rather than attach the file, in order to have it integrated into phabricator diff view. Or use the arcanist command-line tool, which should do it for you.

Jun 13 2016, 11:00 AM · Restricted Project

Jan 15 2016

prados awarded D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk a Like token.
Jan 15 2016, 3:16 AM

Dec 22 2015

berenm added a reviewer for D15717: [clang-format-vs] Use NuGet to download VS plugin dependencies.: klimek.
Dec 22 2015, 8:48 AM
berenm retitled D15717: [clang-format-vs] Use NuGet to download VS plugin dependencies. from to [clang-format-vs] Use NuGet to download VS plugin dependencies..
Dec 22 2015, 8:48 AM
berenm added a comment to D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.

Actually, the main issue that I see is the difficulty to build the plugin itself. The project does not build out-of-the-box on VS2013, and I had to change the assembly references by hand.

Dec 22 2015, 6:02 AM
berenm added a comment to D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.

For information, I just tested the current diff under Visual Studio 2010 SP1 and everything looks fine as well.

Dec 22 2015, 5:55 AM

Dec 14 2015

berenm added inline comments to D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.
Dec 14 2015, 7:39 AM
berenm updated the diff for D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.
  • Patch rebased on trunk
  • Small cleanups in the RunningDocumentTable usage
  • Successfully tested on VS >= 2012
Dec 14 2015, 7:32 AM

Dec 1 2015

berenm added a comment to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

I don't have any compilation error with GCC 5.2.1. I tried again with Clang 3.8 and indeed there is this error:

Dec 1 2015, 5:46 AM

Nov 28 2015

berenm added a comment to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

I don't have commit access, if you don't mind landing it for me :)

Nov 28 2015, 1:05 PM

Nov 27 2015

berenm added inline comments to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).
Nov 27 2015, 5:53 AM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

Fix continue statements.

Nov 27 2015, 5:50 AM
berenm added inline comments to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).
Nov 27 2015, 5:38 AM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

[clang-format] Code cleanup and variable naming in WhitespaceManager::AlignTokens

Nov 27 2015, 5:31 AM

Nov 26 2015

berenm added a comment to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

Updated the diff with a fix for the issue reported in http://lists.llvm.org/pipermail/cfe-dev/2015-November/046057.html

Nov 26 2015, 1:26 PM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

[clang-format] alignConsecutiveXXX: replace the buggy paren / braces counter with a better scope tracker.

Nov 26 2015, 1:24 PM

Nov 18 2015

berenm added a comment to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

Ping?

Nov 18 2015, 10:54 AM

Nov 6 2015

berenm added inline comments to D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.
Nov 6 2015, 7:40 AM

Nov 4 2015

berenm added inline comments to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).
Nov 4 2015, 3:16 PM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

[clang-format] Replace TokenSequenceAligner with simple static functions.

Nov 4 2015, 3:15 PM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

[clang-format] Remove deleted methods declaration.

Nov 4 2015, 11:38 AM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

[clang-format] Alignment code factorization.

Nov 4 2015, 11:35 AM
berenm added a comment to D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

I've also added a fix for the other issue reported on the same bug. I could split the two reviews if necessary.

Nov 4 2015, 7:00 AM
berenm updated the diff for D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).

[clang-format] Count the number of braces and parens on the line instead of remembering only one.

Nov 4 2015, 6:59 AM
berenm updated D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).
Nov 4 2015, 2:19 AM
berenm retitled D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329) from to [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329).
Nov 4 2015, 2:18 AM

Oct 7 2015

berenm added inline comments to D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments..
Oct 7 2015, 8:06 AM
berenm retitled D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments. from to [clang-format] Stop alignment sequences on open braces and parens when aligning assignments..
Oct 7 2015, 7:42 AM

Oct 1 2015

berenm added a comment to D12362: [clang-format] Add support of consecutive declarations alignment.

Thanks, I don't have commit access.

Oct 1 2015, 2:57 AM
berenm added a comment to D12362: [clang-format] Add support of consecutive declarations alignment.
Oct 1 2015, 2:43 AM
berenm added inline comments to D12362: [clang-format] Add support of consecutive declarations alignment.
Oct 1 2015, 2:43 AM
berenm updated subscribers of D12362: [clang-format] Add support of consecutive declarations alignment.
Oct 1 2015, 2:40 AM
berenm updated the diff for D12362: [clang-format] Add support of consecutive declarations alignment.

Fix arcanist insisting on creating the diff against old revision...

Oct 1 2015, 2:39 AM
berenm updated the diff for D12362: [clang-format] Add support of consecutive declarations alignment.
  • Add support of ColumnLimit restrition in AlignConsecutiveAssignments and AlignConsecutiveDeclarations.
Oct 1 2015, 2:38 AM
berenm added a comment to D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Alright, thank you for your time!

Oct 1 2015, 2:36 AM

Sep 29 2015

berenm added inline comments to D12362: [clang-format] Add support of consecutive declarations alignment.
Sep 29 2015, 9:09 AM
berenm added a comment to D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 29 2015, 7:33 AM
berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Fix arcanist messup...

Sep 29 2015, 7:31 AM
berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Address the latest comments, as well as useless code removal.

  • Only the start of the token range is stored, so there is no need to care about the end of the range. Remove the code that was trying to match the end of the range (template, nested name).
  • Add some spaces in test cases (destructors, templates) just to be sure that everything works as expected.
  • Add a missing CHECK-FIXES in the test case on the abstract_class destructor.
Sep 29 2015, 7:29 AM
berenm added inline comments to D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 29 2015, 7:00 AM
berenm updated the diff for D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.

Update the diff with the ProvideAutoLoad attribute.

Sep 29 2015, 2:07 AM

Sep 27 2015

berenm added a comment to D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 27 2015, 2:29 AM
berenm abandoned D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros.

Actually I believe it's better fixed by D13081.

Sep 27 2015, 2:28 AM
berenm added a comment to D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

I think the latest version also addresses D13090 in a better way. The check will now let clang-tidy decide whether warnings and fixes are displayed, whether it is in or from system/user header files or in main file, and will only worry about not emitting warnings and fixes if a macro is involved in the declaration or any usage.

Sep 27 2015, 2:27 AM
berenm added inline comments to D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 27 2015, 2:25 AM
berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Reworked the patches a little bit, addressing the comments and adding
better handling of checks in header files with corresponding unit test cases.

Sep 27 2015, 2:19 AM
berenm added a comment to D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.

Please tell me, if you need me to commit the patch for you after you address the comments.

Yes please, I cannot commit it myself.

Sep 27 2015, 1:24 AM
berenm updated the diff for D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.

Added comments and reformated NamingCheckIdentifer.h with clang-format.

Sep 27 2015, 1:22 AM

Sep 24 2015

berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
  • Do not check for identifier names from system headers
  • Check for SourceLocation validity before modifying them
Sep 24 2015, 1:50 PM
berenm added a comment to D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros.

This will also disable all warnings for declaration / usages outside of the main file.

Sep 24 2015, 1:10 PM
berenm updated the diff for D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.

Reformatting with clang-format

Sep 24 2015, 1:00 PM
berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Update the diff with more context.

Sep 24 2015, 10:42 AM
berenm updated the diff for D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros.

Update the diff with more context, thanks to arcanist.

Sep 24 2015, 10:37 AM
berenm added a comment to D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.
Sep 24 2015, 10:33 AM
berenm updated the diff for D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.

Changed Usages to RawUsageLocs, as DenseSet storing the raw encoding of SourceLocation instead of a vector or SourceRange.

Sep 24 2015, 10:32 AM
berenm added a comment to D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.

Well, the format on save setting is disabled by default, do you mean you had to enable it twice? Or you had it enabled without the C++ development tools, and after the installation you had to disable and enable it again?

Sep 24 2015, 5:04 AM

Sep 23 2015

berenm added a comment to D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk.

Ping? Just to be sure it's not going to fall to oblivion. :)

Sep 23 2015, 1:08 AM
berenm added inline comments to D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.
Sep 23 2015, 12:49 AM
berenm updated the diff for D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros.

Even better with the corresponding unit test fix.

Sep 23 2015, 12:40 AM
berenm retitled D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros from to [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros.
Sep 23 2015, 12:25 AM

Sep 22 2015

berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Fix incorrect line removal in unit test.

Sep 22 2015, 11:45 PM
berenm added a comment to D12915: Identifier Naming clang-tidy check now renames base classes in class declarations where required..

@jbcoe Sorry for making this patch irrelevant but I managed to have the generic approach working in http://reviews.llvm.org/D13081, and I think it should cover this case as well. Thanks anyway for your work, and please keep contributing if you feel the check isn't perfect!

Sep 22 2015, 11:41 PM
berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.

Remove remaining commented code.

Sep 22 2015, 11:35 PM
berenm added a reviewer for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck: alexfh.
Sep 22 2015, 11:34 PM
berenm added a reviewer for D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck: alexfh.
Sep 22 2015, 11:33 PM
berenm updated D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.
Sep 22 2015, 11:32 PM
berenm updated D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 22 2015, 11:31 PM
berenm retitled D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck from [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck. to [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck.
Sep 22 2015, 11:30 PM
berenm changed the visibility for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 22 2015, 11:29 PM
berenm updated the diff for D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 22 2015, 11:29 PM
berenm retitled D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck from to [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck.
Sep 22 2015, 5:24 PM
berenm retitled D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck from to [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck..
Sep 22 2015, 4:43 PM
berenm added a comment to D12915: Identifier Naming clang-tidy check now renames base classes in class declarations where required..

I think that a more generic approach might be better, as discussed in http://reviews.llvm.org/D10933#225746. It should cover this particular case as well as other cases where class names are referenced.

Sep 22 2015, 12:55 PM
berenm added a comment to D12362: [clang-format] Add support of consecutive declarations alignment.

The unit tests should work fine, now that D12369 has been merged.

Sep 22 2015, 12:03 PM

Sep 14 2015

berenm added a comment to D12369: [clang-format] Fix alignConsecutiveAssignments not working properly.

I don't have credentials to commit it, anybody could do it for me if the diff is ok?

Sep 14 2015, 9:03 AM