Page MenuHomePhabricator

[clang-format] Reference qualifiers in member templates causing extra indentation.
ClosedPublic

Authored by AndWass on Sep 25 2019, 11:30 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

AndWass created this revision.Sep 25 2019, 11:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2019, 11:30 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
AndWass changed the repository for this revision from rC Clang to rCTE Clang Tools Extra.Sep 26 2019, 1:02 AM
klimek accepted this revision.Sep 26 2019, 1:10 AM

LG, nice patch, thanks!

This revision is now accepted and ready to land.Sep 26 2019, 1:10 AM

@klimek I don't have commit access yet, could you please commit it for me? Thanks

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2019, 2:24 AM
This revision is now accepted and ready to land.Sep 27 2019, 2:51 AM
AndWass updated this revision to Diff 222287.EditedSep 28 2019, 2:37 AM

Updated to fix test failures.

As the number of clauses is vast, I think sometimes its worth just putting a one-line comment in to explain what construct you are matching, just to help future maintainers

clang/lib/Format/TokenAnnotator.cpp
1375 ↗(On Diff #222287)

could this be done using the MatchingParen?

2704 ↗(On Diff #222287)

are you not testing the volatile case? could you add a comment here as to what you are actually matching like the lines above

lebedev.ri retitled this revision from Reference qualifiers in member templates causing extra indentation. to [clang-format] Reference qualifiers in member templates causing extra indentation..Sep 28 2019, 5:51 AM
AndWass updated this revision to Diff 222299.Sep 28 2019, 12:34 PM

Removed the manual paranthesis-tracking, using NestingLevel instead.
Moved tests to the already existing UnderstandsFunctionRefQualification test case.
Clarified what is being matched against in TokenAnnotator::spaceRequiredBetween

AndWass marked 4 inline comments as done.Sep 28 2019, 12:48 PM
AndWass added inline comments.
clang/lib/Format/TokenAnnotator.cpp
1375 ↗(On Diff #222287)

Removed the old code to use Current.NestingLevel instead.

2704 ↗(On Diff #222287)

const and volatile are tested in existing tests. I moved my tests to the same test case.

AndWass marked 2 inline comments as done.Sep 29 2019, 4:46 AM

Thanks! @MyDeveloperDay could you commit as well? Don't have access for that yet

This revision was automatically updated to reflect the committed changes.