[PATCH] D41487: [clang-format] Adds a FormatStyleSet

Benjamin Kramer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 8 07:41:32 PST 2018


bkramer added inline comments.


================
Comment at: include/clang/Format/Format.h:1700
+    typedef std::map<FormatStyle::LanguageKind,
+                     std::unique_ptr<const FormatStyle>>
+        MapType;
----------------
Why unique_ptr?


================
Comment at: include/clang/Format/Format.h:1706
+    // FormatStyleSet.
+    void Add(FormatStyle Style);
+    // Clears this FormatStyleSet.
----------------
Document what happens if the Style for this language is in the set already.


================
Comment at: lib/Format/Format.cpp:904
+    DefaultStyle.Language = Language;
+    StyleSet.Add(DefaultStyle);
+  }
----------------
std::move


================
Comment at: lib/Format/Format.cpp:939
+    Styles = std::make_shared<MapType>();
+  (*Styles)[Style.Language].reset(new FormatStyle(Style));
+}
----------------
You can std::move `Style` here, it's passed by value.


================
Comment at: lib/Format/Format.cpp:939
+    Styles = std::make_shared<MapType>();
+  (*Styles)[Style.Language].reset(new FormatStyle(Style));
+}
----------------
bkramer wrote:
> You can std::move `Style` here, it's passed by value.
Can Style.Language ever be LK_None? Does that even make sense?


Repository:
  rC Clang

https://reviews.llvm.org/D41487





More information about the cfe-commits mailing list