From 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc Mon Sep 17 00:00:00 2001
From: owenca <owenpiano@gmail.com>
Date: Sat, 28 Aug 2021 15:21:23 -0700
Subject: [PATCH] [clang-format] Add PackConstructorInitializers backward
 compat test

Add backward compatibility tests for mapping the deprecated
ConstructorInitializerAllOnOneLineOrOnePerLine and
AllowAllConstructorInitializersOnNextLine to
PackConstructorInitializers.

Differential Revision: https://reviews.llvm.org/D108882
---
 clang/lib/Format/Format.cpp           | 25 +++++++++++++++++--------
 clang/unittests/Format/FormatTest.cpp | 16 ++++++++++++++++
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 78a92aa54200..ecc6d3f3a47e 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -658,7 +658,14 @@ template <> struct MappingTraits<FormatStyle> {
 
     IO.mapOptional("PackConstructorInitializers",
                    Style.PackConstructorInitializers);
-    // For backward compatibility.
+    // For backward compatibility:
+    // The default value of ConstructorInitializerAllOnOneLineOrOnePerLine was
+    // false unless BasedOnStyle was Google or Chromium whereas that of
+    // AllowAllConstructorInitializersOnNextLine was always true, so the
+    // equivalent default value of PackConstructorInitializers is PCIS_NextLine
+    // for Google/Chromium or PCIS_BinPack otherwise. If the deprecated options
+    // had a non-default value while PackConstructorInitializers has a default
+    // value, set the latter to an equivalent non-default value if needed.
     StringRef BasedOn;
     IO.mapOptional("BasedOnStyle", BasedOn);
     const bool IsGoogleOrChromium = BasedOn.equals_insensitive("google") ||
@@ -668,17 +675,19 @@ template <> struct MappingTraits<FormatStyle> {
     IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
                    OnCurrentLine);
     IO.mapOptional("AllowAllConstructorInitializersOnNextLine", OnNextLine);
-    if (IsGoogleOrChromium &&
-        Style.PackConstructorInitializers == FormatStyle::PCIS_NextLine) {
+    if (!IsGoogleOrChromium) {
+      if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack &&
+          OnCurrentLine) {
+        Style.PackConstructorInitializers = OnNextLine
+                                                ? FormatStyle::PCIS_NextLine
+                                                : FormatStyle::PCIS_CurrentLine;
+      }
+    } else if (Style.PackConstructorInitializers ==
+               FormatStyle::PCIS_NextLine) {
       if (!OnCurrentLine)
         Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
       else if (!OnNextLine)
         Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
-    } else if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack &&
-               OnCurrentLine) {
-      Style.PackConstructorInitializers = OnNextLine
-                                              ? FormatStyle::PCIS_NextLine
-                                              : FormatStyle::PCIS_CurrentLine;
     }
 
     IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 4ca3a22a76c8..45ed0f9de7d3 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18467,6 +18467,22 @@ TEST_F(FormatTest, ParsesConfiguration) {
               PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
   CHECK_PARSE("PackConstructorInitializers: NextLine",
               PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  // For backward compatibility:
+  CHECK_PARSE("BasedOnStyle: Google\n"
+              "ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+              "AllowAllConstructorInitializersOnNextLine: false",
+              PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
+  CHECK_PARSE("BasedOnStyle: Google\n"
+              "ConstructorInitializerAllOnOneLineOrOnePerLine: false",
+              PackConstructorInitializers, FormatStyle::PCIS_BinPack);
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+              "AllowAllConstructorInitializersOnNextLine: true",
+              PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+              "AllowAllConstructorInitializersOnNextLine: false",
+              PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
 
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
   CHECK_PARSE("EmptyLineBeforeAccessModifier: Never",