[PATCH] D68072: Reference qualifiers in member templates causing extra indentation.

[Andreas Wass via Phabricator via cfe-commits]

AndWass created this revision.
AndWass added reviewers: klimek, owenpan, krasimir, timwoj.
AndWass added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The following code

  struct f {
template 
void bar() && noexcept {}
  };

will be formatted to the following with LLVM style, and
`AlwaysBreakTemplateDeclarations: Yes`

  struct f {
template 
void bar() && noexcept {}
  };

The indentation of the `void bar()` line is wrong.


Repository:
  rC Clang

https://reviews.llvm.org/D68072

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14347,6 +14347,41 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, RefQualifiedTemplateMember) {
+  FormatStyle AlignStyle = getLLVMStyle();
+  AlignStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() & noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() && noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() const & noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() const & noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  auto foo() && noexcept -> int & {}\n"
+   "};",
+   AlignStyle);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -65,7 +65,7 @@
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-Keywords(Keywords) {
+TrailingReturnFound(false), Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1389,7 +1389,10 @@
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0) {
   Current.Type = TT_TrailingReturnArrow;
-} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+  TrailingReturnFound = true;
+} else if (Current.is(tok::star) ||
+   (Current.isOneOf(tok::amp, tok::ampamp) &&
+(!Line.MightBeFunctionDecl || TrailingReturnFound))) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
Contexts.back().IsExpression,
@@ -1412,6 +1415,8 @@
 Current.Type = TT_ConditionalExpr;
   }
 } else if (Current.isBinaryOperator() &&
+   !(Line.MightBeFunctionDecl &&
+ Current.isOneOf(tok::amp, tok::ampamp)) &&
(!Current.Previous || Current.Previous->isNot(tok::l_square)) &&
(!Current.is(tok::greater) &&
 Style.Language != FormatStyle::LK_TextProto)) {
@@ -1486,10 +1491,12 @@
   // colon after this, this is the only place which annotates the identifier
   // as a selector.)
   Current.Type = TT_SelectorName;
-} else if (Current.isOneOf(tok::identifier, tok::kw_const) &&
+} else if (Current.isOneOf(tok::identifier, tok::kw_const, tok::amp,
+   tok::ampamp) &&
Current.Previous &&
!Current.Previous->isOneOf(tok::equal, tok::at) &&
-   Line.MightBeFunctionDecl && Contexts.size() == 1) {
+   Line.MightBeFunctionDecl && !TrailingReturnFound &&
+   Contexts.size() == 1) {
   // Line.MightBeFunctionDecl can only be true after the parentheses of a
   // function declaration have been found.
   Current.Type = TT_TrailingAnnotation;
@@ -1767,6 +1774,7 @@
   AnnotatedLine &Line;
   FormatToken *CurrentToken;
   bool AutoFound;
+  bool TrailingReturnFound;
   const AdditionalKeywords &Keywords;
 
   // Set of "<" tokens that do not open a template parameter list. If parseAngle
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68072: Reference qualifiers in member templates causing extra indentation.

[Andreas Wass via Phabricator via cfe-commits]

AndWass added a comment.

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


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68072/new/

https://reviews.llvm.org/D68072



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68072: Reference qualifiers in member templates causing extra indentation.

[Andreas Wass via Phabricator via cfe-commits]

AndWass updated this revision to Diff 87.
AndWass added a comment.

Updated to fix test failures.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68072/new/

https://reviews.llvm.org/D68072

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14373,6 +14373,41 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, RefQualifiedTemplateMember) {
+  FormatStyle AlignStyle = getLLVMStyle();
+  AlignStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() & noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() && noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() const & noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo() const & noexcept {}\n"
+   "};",
+   AlignStyle);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  auto foo() && noexcept -> int & {}\n"
+   "};",
+   AlignStyle);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -65,7 +65,7 @@
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-Keywords(Keywords) {
+TrailingReturnFound(false), ParenthesisDepth(0), Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1367,6 +1367,12 @@
   }
 }
 
+if (Current.is(tok::l_paren)) {
+  ParenthesisDepth++;
+} else if (Current.is(tok::r_paren)) {
+  ParenthesisDepth--;
+}
+
 // Line.MightBeFunctionDecl can only be true after the parentheses of a
 // function declaration have been found. In this case, 'Current' is a
 // trailing token of this declaration and thus cannot be a name.
@@ -1389,7 +1395,11 @@
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0) {
   Current.Type = TT_TrailingReturnArrow;
-} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+  TrailingReturnFound = true;
+} else if (Current.is(tok::star) ||
+   (Current.isOneOf(tok::amp, tok::ampamp) &&
+(ParenthesisDepth != 0 || !Line.MightBeFunctionDecl ||
+ TrailingReturnFound))) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
Contexts.back().IsExpression,
@@ -1412,6 +1422,8 @@
 Current.Type = TT_ConditionalExpr;
   }
 } else if (Current.isBinaryOperator() &&
+   !(Line.MightBeFunctionDecl && ParenthesisDepth == 0 &&
+ Current.isOneOf(tok::amp, tok::ampamp)) &&
(!Current.Previous || Current.Previous->isNot(tok::l_square)) &&
(!Current.is(tok::greater) &&
 Style.Language != FormatStyle::LK_TextProto)) {
@@ -1486,10 +1498,12 @@
   // colon after this, this is the only place which annotates the identifier
   // as a selector.)
   Current.Type = TT_SelectorName;
-} else if (Current.isOneOf(tok::identifier, tok::kw_const) &&
+} else if (Current.isOneOf(tok::identifier, tok::kw_const, tok::amp,
+   tok::ampamp) &&
Current.Previous &&
!Current.Previous->isOneOf(tok::equal, tok::at) &&
-   Line.MightBeFunctionDecl && Contexts.size() == 1) {
+   Line.MightBeFunctionDecl && !TrailingReturnFound &&
+   Contexts.size() == 1 && ParenthesisDepth == 0) {
   // Line.MightBeFunctionDecl can only be true after the parentheses of a
   // function declaration have been found.
   Current.Type = TT_TrailingAnnotation;
@@ -1767,6 +1781,8 @@
   AnnotatedLine &Line;
   FormatToken *CurrentToken;
   bool AutoFound;
+  bool TrailingReturnFound;
+  int ParenthesisDepth;
   const AdditionalKeywords &Keywords;
 
   // Set of "<" tokens that do not open a template parameter list. If parseAngle
@@ -2680,6

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

[Andreas Wass via Phabricator via cfe-commits]

AndWass updated this revision to Diff 99.
AndWass added a comment.

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`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68072/new/

https://reviews.llvm.org/D68072

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6901,6 +6901,73 @@
Spaces);
   verifyFormat("Deleted &operator=( const Deleted & ) &;", Spaces);
   verifyFormat("SomeType MemberFunction( const Deleted & ) &;", Spaces);
+
+  FormatStyle BreakTemplate = getLLVMStyle();
+  BreakTemplate.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) & noexcept {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) && noexcept {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) const & noexcept {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int &foo(const std::string &str) const & noexcept {}\n"
+   "};",
+   BreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  auto foo(const std::string &str) && noexcept -> int & {}\n"
+   "};",
+   BreakTemplate);
+
+  FormatStyle AlignLeftBreakTemplate = getLLVMStyle();
+  AlignLeftBreakTemplate.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+  AlignLeftBreakTemplate.PointerAlignment = FormatStyle::PAS_Left;
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int& foo(const std::string& str) & noexcept {}\n"
+   "};",
+   AlignLeftBreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int& foo(const std::string& str) && noexcept {}\n"
+   "};",
+   AlignLeftBreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int& foo(const std::string& str) const & noexcept {}\n"
+   "};",
+   AlignLeftBreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  int& foo(const std::string& str) const & noexcept {}\n"
+   "};",
+   AlignLeftBreakTemplate);
+
+  verifyFormat("struct f {\n"
+   "  template \n"
+   "  auto foo(const std::string& str) && noexcept -> int& {}\n"
+   "};",
+   AlignLeftBreakTemplate);
 }
 
 TEST_F(FormatTest, UnderstandsNewAndDelete) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -65,7 +65,7 @@
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
const AdditionalKeywords &Keywords)
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
-Keywords(Keywords) {
+TrailingReturnFound(false), Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
 resetTokenMetadata(CurrentToken);
   }
@@ -1389,7 +1389,11 @@
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0) {
   Current.Type = TT_TrailingReturnArrow;
-} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+  TrailingReturnFound = true;
+} else if (Current.is(tok::star) ||
+   (Current.isOneOf(tok::amp, tok::ampamp) &&
+(Current.NestingLevel != 0 || !Line.MightBeFunctionDecl ||
+ TrailingReturnFound))) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
Contexts.back().IsExpression,
@@ -1412,6 +1416,8 @@
 Current.Type = TT_ConditionalExpr;
   }
 } else if (Current.isBinaryOperator() &&
+   !(Line.MightBeFunctionDecl && Current.NestingLevel == 0 &&
+ Current.isOneOf(tok::amp, tok::ampamp)) &&
(!Current.Previous || Current.Previous->isNot(tok::l_square)) &&
(!Current.is(tok::greater) &&
 Style.Languag

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

[Andreas Wass via Phabricator via cfe-commits]

AndWass marked 4 inline comments as done.
AndWass added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1375
+}
+
 // Line.MightBeFunctionDecl can only be true after the parentheses of a

MyDeveloperDay wrote:
> could this be done using the MatchingParen?
Removed the old code to use `Current.NestingLevel` instead.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2704
+return Style.PointerAlignment != FormatStyle::PAS_Left;
   return true;
 }

MyDeveloperDay wrote:
>  are you not testing the volatile case? could you add a comment here as to 
> what you are actually matching like the lines above
const and volatile are tested in existing tests. I moved my tests to the same 
test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68072/new/

https://reviews.llvm.org/D68072



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

[Andreas Wass via Phabricator via cfe-commits]

AndWass marked 2 inline comments as done.
AndWass added a comment.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68072/new/

https://reviews.llvm.org/D68072



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits