PR-134337: fixed PR comments

Author: Ivan Rymarchyk

clang/docs/ClangFormatStyleOptions.rst

@@ -1994,28 +1994,31 @@ the configuration (without a prefix: ``Auto``).
       Inline: true
       All: false
 
-  * ``bool Empty`` Only merge empty functions.
+  * ``bool Empty`` Merge top-level empty functions.
 
     .. code-block:: c++
 
       void f() {}
       void f2() {
         bar2();
       }
+      void f3() { /* comment */ }
 
-  * ``bool Inline`` Only merge functions defined inside a class.
+  * ``bool Inline`` Merge functions defined inside a class.
 
     .. code-block:: c++
 
       class Foo {
         void f() { foo(); }
+        void g() {}
       };
       void f() {
         foo();
       }
-      void f() {}
+      void f() {
+      }
 
-  * ``bool All`` Merge all functions fitting on a single line.
+  * ``bool Other`` Merge all functions fitting on a single line.
 
     .. code-block:: c++
 

clang/include/clang/Format/Format.h

@@ -890,23 +890,26 @@ struct FormatStyle {
   ///     All: false
   /// \endcode
   struct ShortFunctionMergeFlags {
-    /// Only merge empty functions.
+    /// Merge top-level empty functions.
     /// \code
     ///   void f() {}
     ///   void f2() {
     ///     bar2();
     ///   }
+    ///   void f3() { /* comment */ }
     /// \endcode
     bool Empty;
-    /// Only merge functions defined inside a class.
+    /// Merge functions defined inside a class.
     /// \code
     ///   class Foo {
     ///     void f() { foo(); }
+    ///     void g() {}
     ///   };
     ///   void f() {
     ///     foo();
     ///   }
-    ///   void f() {}
+    ///   void f() {
+    ///   }
     /// \endcode
     bool Inline;
     /// Merge all functions fitting on a single line.
@@ -916,15 +919,15 @@ struct FormatStyle {
     ///   };
     ///   void f() { bar(); }
     /// \endcode
-    bool All;
+    bool Other;
 
-    ShortFunctionMergeFlags() : Empty(false), Inline(false), All(false) {}
+    ShortFunctionMergeFlags() : Empty(false), Inline(false), Other(false) {}
 
     ShortFunctionMergeFlags(bool Empty, bool Inline, bool All)
-        : Empty(Empty), Inline(Inline), All(All) {}
+        : Empty(Empty), Inline(Inline), Other(All) {}
 
     bool operator==(const ShortFunctionMergeFlags &R) const {
-      return Empty == R.Empty && Inline == R.Inline && All == R.All;
+      return Empty == R.Empty && Inline == R.Inline && Other == R.Other;
     }
     bool operator!=(const ShortFunctionMergeFlags &R) const {
       return !(*this == R);

clang/lib/Format/Format.cpp

@@ -630,7 +630,7 @@ template <> struct MappingTraits<FormatStyle::ShortFunctionMergeFlags> {
   static void mapping(IO &IO, FormatStyle::ShortFunctionMergeFlags &Value) {
     IO.mapOptional("Empty", Value.Empty);
     IO.mapOptional("Inline", Value.Inline);
-    IO.mapOptional("All", Value.All);
+    IO.mapOptional("Other", Value.Other);
   }
 };
 
@@ -1504,7 +1504,7 @@ static void expandPresetsShortFunctionsOnSingleLine(FormatStyle &Expanded) {
   }
 
   if (Expanded.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
-    Expanded.AllowShortFunctionsOnASingleLineOptions.All = true;
+    Expanded.AllowShortFunctionsOnASingleLineOptions.Other = true;
 }
 
 FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
@@ -1537,7 +1537,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AllowShortEnumsOnASingleLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortFunctionsOnASingleLineOptions = {};
-  LLVMStyle.AllowShortFunctionsOnASingleLineOptions.All = true;
+  LLVMStyle.AllowShortFunctionsOnASingleLineOptions.Other = true;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;

clang/lib/Format/TokenAnnotator.cpp

@@ -5688,7 +5688,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
         !Left.Children.empty()) {
       // Support AllowShortFunctionsOnASingleLine for JavaScript.
       return (!Style.AllowShortFunctionsOnASingleLineOptions.Inline &&
-              !Style.AllowShortFunctionsOnASingleLineOptions.All) ||
+              !Style.AllowShortFunctionsOnASingleLineOptions.Other) ||
              (Left.NestingLevel == 0 && Line.Level == 0 &&
               Style.AllowShortFunctionsOnASingleLineOptions.Inline);
     }

clang/lib/Format/UnwrappedLineFormatter.cpp

@@ -293,7 +293,7 @@ class LineJoiner {
 
     auto ShouldMergeShortFunctions = [this, &I, &NextLine, PreviousLine,
                                       TheLine]() {
-      if (Style.AllowShortFunctionsOnASingleLineOptions.All)
+      if (Style.AllowShortFunctionsOnASingleLineOptions.Other)
         return true;
       if (Style.AllowShortFunctionsOnASingleLineOptions.Empty &&
           NextLine.First->is(tok::r_brace)) {

clang/unittests/Format/ConfigParseTest.cpp

@@ -258,7 +258,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
   CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, Other);
   CHECK_PARSE_NESTED_BOOL(AllowShortFunctionsOnASingleLineOptions, Empty);
   CHECK_PARSE_NESTED_BOOL(AllowShortFunctionsOnASingleLineOptions, Inline);
-  CHECK_PARSE_NESTED_BOOL(AllowShortFunctionsOnASingleLineOptions, All);
+  CHECK_PARSE_NESTED_BOOL(AllowShortFunctionsOnASingleLineOptions, Other);
 }
 
 #undef CHECK_PARSE_BOOL

clang/unittests/Format/FormatTest.cpp

@@ -15125,7 +15125,7 @@ TEST_F(FormatTest, CustomShortFunctionOptions) {
   CustomEmpty.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Custom;
   CustomEmpty.AllowShortFunctionsOnASingleLineOptions.Empty = true;
   CustomEmpty.AllowShortFunctionsOnASingleLineOptions.Inline = false;
-  CustomEmpty.AllowShortFunctionsOnASingleLineOptions.All = false;
+  CustomEmpty.AllowShortFunctionsOnASingleLineOptions.Other = false;
 
   // Empty functions should be on a single line
   verifyFormat("int f() {}", CustomEmpty);
@@ -15146,6 +15146,9 @@ TEST_F(FormatTest, CustomShortFunctionOptions) {
                "};",
                CustomEmpty);
 
+  // test with comment
+  verifyFormat("void f3() { /* comment */ }", CustomEmpty);
+
   // Test with AfterFunction = true
   CustomEmpty.BreakBeforeBraces = FormatStyle::BS_Custom;
   CustomEmpty.BraceWrapping.AfterFunction = true;
@@ -15161,7 +15164,7 @@ TEST_F(FormatTest, CustomShortFunctionOptions) {
   CustomInline.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Custom;
   CustomInline.AllowShortFunctionsOnASingleLineOptions.Empty = false;
   CustomInline.AllowShortFunctionsOnASingleLineOptions.Inline = true;
-  CustomInline.AllowShortFunctionsOnASingleLineOptions.All = false;
+  CustomInline.AllowShortFunctionsOnASingleLineOptions.Other = false;
 
   verifyFormat("class C {\n"
                "  int f() {}\n"
@@ -15188,7 +15191,7 @@ TEST_F(FormatTest, CustomShortFunctionOptions) {
   CustomAll.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Custom;
   CustomAll.AllowShortFunctionsOnASingleLineOptions.Empty = false;
   CustomAll.AllowShortFunctionsOnASingleLineOptions.Inline = false;
-  CustomAll.AllowShortFunctionsOnASingleLineOptions.All = true;
+  CustomAll.AllowShortFunctionsOnASingleLineOptions.Other = true;
 
   // All functions should be on a single line if they fit
   verifyFormat("int f() { return 42; }", CustomAll);
@@ -15209,7 +15212,7 @@ TEST_F(FormatTest, CustomShortFunctionOptions) {
   CustomMixed.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Custom;
   CustomMixed.AllowShortFunctionsOnASingleLineOptions.Empty = true;
   CustomMixed.AllowShortFunctionsOnASingleLineOptions.Inline = true;
-  CustomMixed.AllowShortFunctionsOnASingleLineOptions.All = false;
+  CustomMixed.AllowShortFunctionsOnASingleLineOptions.Other = false;
 
   // Empty functions should be on a single line
   verifyFormat("int f() {}", CustomMixed);