[PATCH] D149643: Correctly limit formatted ranges when specifying qualifier alignment

[Colin Ogilvie via Phabricator via cfe-commits]

cogilvie created this revision.
cogilvie added reviewers: MyDeveloperDay, rymiel, HazardyKnusperkeks, owenpan.
cogilvie added a project: clang-format.
Herald added projects: All, clang.
cogilvie requested review of this revision.

The qualifier alignment fixer appeared to ignore any ranges specified for 
limiting formatting.
This change ensures that it only formats affected lines to avoid unexpected 
changes as reported by:
https://github.com/llvm/llvm-project/issues/54888


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149643

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===================================================================
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -1343,6 +1343,31 @@
                "TemplateType<Container const> t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted the second should remain as is
+  EXPECT_EQ("template <typename T> const Foo f();\n"
+            "template <typename T> Foo const f();",
+            format("template <typename T> Foo const f();\n"
+                   "template <typename T> Foo const f();",
+                   Style, SC_ExpectComplete,
+                   std::vector<tooling::Range>(1, tooling::Range(0, 36))));
+
+  // Only the middle line should be formatted the first and last should remain
+  // as is
+  EXPECT_EQ("template <typename T> Foo const f();\n"
+            "template <typename T> const Foo f();\n"
+            "template <typename T> Foo const f();",
+            format("template <typename T> Foo const f();\n"
+                   "template <typename T> Foo const f();\n"
+                   "template <typename T> Foo const f();",
+                   Style, SC_ExpectComplete,
+                   std::vector<tooling::Range>(1, tooling::Range(37, 36))));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===================================================================
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -587,7 +587,7 @@
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-    if (Line->InPPDirective)
+    if (!Line->Affected || Line->InPPDirective)
       continue;
     FormatToken *First = Line->First;
     assert(First);



Index: clang/unittests/Format/QualifierFixerTest.cpp
===================================================================
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -1343,6 +1343,31 @@
                "TemplateType<Container const> t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted the second should remain as is
+  EXPECT_EQ("template <typename T> const Foo f();\n"
+            "template <typename T> Foo const f();",
+            format("template <typename T> Foo const f();\n"
+                   "template <typename T> Foo const f();",
+                   Style, SC_ExpectComplete,
+                   std::vector<tooling::Range>(1, tooling::Range(0, 36))));
+
+  // Only the middle line should be formatted the first and last should remain
+  // as is
+  EXPECT_EQ("template <typename T> Foo const f();\n"
+            "template <typename T> const Foo f();\n"
+            "template <typename T> Foo const f();",
+            format("template <typename T> Foo const f();\n"
+                   "template <typename T> Foo const f();\n"
+                   "template <typename T> Foo const f();",
+                   Style, SC_ExpectComplete,
+                   std::vector<tooling::Range>(1, tooling::Range(37, 36))));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===================================================================
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -587,7 +587,7 @@
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-    if (Line->InPPDirective)
+    if (!Line->Affected || Line->InPPDirective)
       continue;
     FormatToken *First = Line->First;
     assert(First);

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