[clang] [clang-format] Remove YAML hack to emit a BasedOnStyle comment (PR #89228)

Jannik Silvanus via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 23 03:34:41 PDT 2024


https://github.com/jasilvanus updated https://github.com/llvm/llvm-project/pull/89228

>From 5d4a3b0f922b0c28960f610c695c92da7d3538c1 Mon Sep 17 00:00:00 2001
From: Jannik Silvanus <jannik.silvanus at amd.com>
Date: Thu, 18 Apr 2024 14:56:47 +0200
Subject: [PATCH 1/2] [clang-format] Remove YAML hack to emit a BasedOnStyle
 comment

When serializing a formatting style to YAML, we were emitting a
comment `# BasedOnStyle: <style>` if the serialized formatting
style matches one of the known styles. This is useful,
but mis-uses the YAML API.

An upcoming change to fix keys with special characters by quoting them
breaks this, and will emit a non-comment *key* `'# BasedOnStyle': <style>`
instead.
(https://github.com/llvm/llvm-project/pull/88763)

Thus, remove this hack.
Instead, we emit an ordinary key OrigBasedOnStyle that is ignored
when reading back the data. An alternative would be to just remove
it completely.

Ideally, we'd emit a proper comment, but our YAML API doesn't support
that.
---
 clang/lib/Format/Format.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index ccb2c9190e2eff..9b311343387ead 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -807,12 +807,18 @@ template <> struct MappingTraits<FormatStyle> {
         FormatStyle PredefinedStyle;
         if (getPredefinedStyle(StyleName, Style.Language, &PredefinedStyle) &&
             Style == PredefinedStyle) {
-          IO.mapOptional("# BasedOnStyle", StyleName);
+          // For convenience, emit the info which style this matches. However,
+          // setting BasedOnStyle will override all other keys when importing,
+          // so we set a helper key that is ignored when importing.
+          // Ideally, we'd want a YAML comment here, but that's not supported.
+          IO.mapOptional("OrigBasedOnStyle", StyleName);
           BasedOnStyle = StyleName;
           break;
         }
       }
     } else {
+      StringRef OrigBasedOnStyle; // ignored
+      IO.mapOptional("OrigBasedOnStyle", OrigBasedOnStyle);
       IO.mapOptional("BasedOnStyle", BasedOnStyle);
       if (!BasedOnStyle.empty()) {
         FormatStyle::LanguageKind OldLanguage = Style.Language;

>From 8d88b9dd62cb8c89ab841a49cbe49d3d86f1286c Mon Sep 17 00:00:00 2001
From: Jannik Silvanus <jannik.silvanus at amd.com>
Date: Tue, 23 Apr 2024 12:34:18 +0200
Subject: [PATCH 2/2] [clang-format] Remove dummy field

This may cause more confusion than helping.
If we really need a comment, we can add YAML
support for comments and add the comment back.
---
 clang/lib/Format/Format.cpp | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 9b311343387ead..373dd4e60bf33f 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -807,18 +807,11 @@ template <> struct MappingTraits<FormatStyle> {
         FormatStyle PredefinedStyle;
         if (getPredefinedStyle(StyleName, Style.Language, &PredefinedStyle) &&
             Style == PredefinedStyle) {
-          // For convenience, emit the info which style this matches. However,
-          // setting BasedOnStyle will override all other keys when importing,
-          // so we set a helper key that is ignored when importing.
-          // Ideally, we'd want a YAML comment here, but that's not supported.
-          IO.mapOptional("OrigBasedOnStyle", StyleName);
           BasedOnStyle = StyleName;
           break;
         }
       }
     } else {
-      StringRef OrigBasedOnStyle; // ignored
-      IO.mapOptional("OrigBasedOnStyle", OrigBasedOnStyle);
       IO.mapOptional("BasedOnStyle", BasedOnStyle);
       if (!BasedOnStyle.empty()) {
         FormatStyle::LanguageKind OldLanguage = Style.Language;



More information about the cfe-commits mailing list