r181326 - Config file support for clang-format, part 1.

Alexander Kornienko alexfh at google.com
Thu May 9 18:38:35 PDT 2013


On Thu, May 9, 2013 at 10:24 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Tue, May 7, 2013 at 11:32 AM, Alexander Kornienko <alexfh at google.com>
> wrote:
> > Author: alexfh
> > Date: Tue May  7 10:32:14 2013
> > New Revision: 181326
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=181326&view=rev
> > Log:
> > Config file support for clang-format, part 1.
> >
> > Summary:
> > Added parseConfiguration method, which reads FormatStyle from YAML
> > string. This supports all FormatStyle fields and an additional
> BasedOnStyle
> > field, which can be used to specify base style.
> >
> > Reviewers: djasper, klimek
> >
> > Reviewed By: djasper
> >
> > CC: cfe-commits
> >
> > Differential Revision: http://llvm-reviews.chandlerc.com/D754
> >
> > Modified:
> >     cfe/trunk/include/clang/Format/Format.h
> >     cfe/trunk/lib/Format/Format.cpp
> >     cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > Modified: cfe/trunk/include/clang/Format/Format.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=181326&r1=181325&r2=181326&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Format/Format.h (original)
> > +++ cfe/trunk/include/clang/Format/Format.h Tue May  7 10:32:14 2013
> > @@ -17,6 +17,7 @@
> >
> >  #include "clang/Frontend/FrontendAction.h"
> >  #include "clang/Tooling/Refactoring.h"
> > +#include "llvm/Support/system_error.h"
> >
> >  namespace clang {
> >
> > @@ -110,6 +111,18 @@ FormatStyle getChromiumStyle();
> >  ///
> https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style.
> >  FormatStyle getMozillaStyle();
> >
> > +/// \brief Returns a predefined style by name.
> > +///
> > +/// Currently supported names: LLVM, Google, Chromium, Mozilla. Names
> are
> > +/// compared case-insensitively.
> > +FormatStyle getPredefinedStyle(StringRef Name);
> > +
> > +/// \brief Parse configuration from YAML-formatted text.
> > +llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style);
> > +
> > +/// \brief Gets configuration in a YAML string.
> > +std::string configurationAsText(const FormatStyle &Style);
> > +
> >  /// \brief Reformats the given \p Ranges in the token stream coming out
> of
> >  /// \c Lex.
> >  ///
> >
> > Modified: cfe/trunk/lib/Format/Format.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=181326&r1=181325&r2=181326&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/Format.cpp (original)
> > +++ cfe/trunk/lib/Format/Format.cpp Tue May  7 10:32:14 2013
> > @@ -28,9 +28,59 @@
> >  #include "llvm/ADT/STLExtras.h"
> >  #include "llvm/Support/Allocator.h"
> >  #include "llvm/Support/Debug.h"
> > +#include "llvm/Support/YAMLTraits.h"
> >  #include <queue>
> >  #include <string>
> >
> > +namespace llvm {
> > +namespace yaml {
> > +template <>
> > +struct
> ScalarEnumerationTraits<clang::format::FormatStyle::LanguageStandard> {
> > +  static void enumeration(IO &io,
> > +                          clang::format::FormatStyle::LanguageStandard
> &value) {
> > +    io.enumCase(value, "C++03", clang::format::FormatStyle::LS_Cpp03);
> > +    io.enumCase(value, "C++11", clang::format::FormatStyle::LS_Cpp11);
> > +    io.enumCase(value, "Auto", clang::format::FormatStyle::LS_Auto);
> > +  }
> > +};
> > +
> > +template <> struct MappingTraits<clang::format::FormatStyle> {
> > +  static void mapping(llvm::yaml::IO &IO, clang::format::FormatStyle
> &Style) {
> > +    if (!IO.outputting()) {
> > +      StringRef BasedOnStyle;
> > +      IO.mapOptional("BasedOnStyle", BasedOnStyle);
> > +
> > +      if (!BasedOnStyle.empty())
> > +        Style = clang::format::getPredefinedStyle(BasedOnStyle);
> > +    }
> > +
> > +    IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
> > +    IO.mapOptional("AlignEscapedNewlinesLeft",
> Style.AlignEscapedNewlinesLeft);
> > +    IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
> > +                   Style.AllowAllParametersOfDeclarationOnNextLine);
> > +    IO.mapOptional("AllowShortIfStatementsOnASingleLine",
> > +                   Style.AllowShortIfStatementsOnASingleLine);
> > +    IO.mapOptional("BinPackParameters", Style.BinPackParameters);
> > +    IO.mapOptional("ColumnLimit", Style.ColumnLimit);
> > +    IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
> > +
> Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
> > +    IO.mapOptional("DerivePointerBinding", Style.DerivePointerBinding);
> > +    IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
> > +    IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
> > +    IO.mapOptional("ObjCSpaceBeforeProtocolList",
> > +                   Style.ObjCSpaceBeforeProtocolList);
> > +    IO.mapOptional("PenaltyExcessCharacter",
> Style.PenaltyExcessCharacter);
> > +    IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",
> > +                   Style.PenaltyReturnTypeOnItsOwnLine);
> > +    IO.mapOptional("PointerBindsToType", Style.PointerBindsToType);
> > +    IO.mapOptional("SpacesBeforeTrailingComments",
> > +                   Style.SpacesBeforeTrailingComments);
> > +    IO.mapOptional("Standard", Style.Standard);
> > +  }
> > +};
> > +}
> > +}
> > +
> >  namespace clang {
> >  namespace format {
> >
> > @@ -98,6 +148,37 @@ FormatStyle getMozillaStyle() {
> >    return MozillaStyle;
> >  }
> >
> > +FormatStyle getPredefinedStyle(StringRef Name) {
> > +  if (Name.equals_lower("llvm"))
> > +    return getLLVMStyle();
> > +  if (Name.equals_lower("chromium"))
> > +    return getChromiumStyle();
> > +  if (Name.equals_lower("mozilla"))
> > +    return getMozillaStyle();
> > +  if (Name.equals_lower("google"))
> > +    return getGoogleStyle();
> > +
> > +  llvm::errs() << "Unknown style " << Name << ", using Google style.\n";
> > +  return getGoogleStyle();
> > +}
> > +
> > +llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style)
> {
> > +  llvm::yaml::Input Input(Text);
> > +  Input >> *Style;
> > +  return Input.error();
> > +}
> > +
> > +std::string configurationAsText(const FormatStyle &Style) {
> > +  std::string Text;
> > +  llvm::raw_string_ostream Stream(Text);
> > +  llvm::yaml::Output Output(Stream);
> > +  // We use the same mapping method for input and output, so we need a
> non-const
> > +  // reference here.
> > +  FormatStyle NonConstStyle = Style;
> > +  Output << NonConstStyle;
> > +  return Text;
> > +}
> > +
> >  // Returns the length of everything up to the first possible line break
> after
> >  // the ), ], } or > matching \c Tok.
> >  static unsigned getLengthToMatchingParen(const AnnotatedToken &Tok) {
> >
> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181326&r1=181325&r2=181326&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue May  7 10:32:14 2013
> > @@ -3909,5 +3909,113 @@ TEST_F(FormatTest, DoNotCreateUnreasonab
> >                 "}");
> >  }
> >
> > +bool operator==(const FormatStyle &L, const FormatStyle &R) {
> > +  return L.AccessModifierOffset == R.AccessModifierOffset &&
> > +         L.AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft &&
> > +         L.AllowAllParametersOfDeclarationOnNextLine ==
> > +             R.AllowAllParametersOfDeclarationOnNextLine &&
> > +         L.AllowShortIfStatementsOnASingleLine ==
> > +             R.AllowShortIfStatementsOnASingleLine &&
> > +         L.BinPackParameters == R.BinPackParameters &&
> > +         L.ColumnLimit == R.ColumnLimit &&
> > +         L.ConstructorInitializerAllOnOneLineOrOnePerLine ==
> > +             R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
> > +         L.DerivePointerBinding == R.DerivePointerBinding &&
> > +         L.IndentCaseLabels == R.IndentCaseLabels &&
> > +         L.MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
> > +         L.ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList
> &&
> > +         L.PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
> > +         L.PenaltyReturnTypeOnItsOwnLine ==
> R.PenaltyReturnTypeOnItsOwnLine &&
> > +         L.PointerBindsToType == R.PointerBindsToType &&
> > +         L.SpacesBeforeTrailingComments ==
> R.SpacesBeforeTrailingComments &&
> > +         L.Standard == R.Standard;
> > +}
> > +
> > +bool allStylesEqual(ArrayRef<FormatStyle> Styles) {
> > +  for (size_t i = 1; i < Styles.size(); ++i)
> > +    if (!(Styles[0] == Styles[i]))
> > +      return false;
> > +  return true;
> > +}
> > +
> > +TEST_F(FormatTest, GetsPredefinedStyleByName) {
> > +  FormatStyle LLVMStyles[] = { getLLVMStyle(),
> getPredefinedStyle("LLVM"),
> > +                               getPredefinedStyle("llvm"),
> > +                               getPredefinedStyle("lLvM") };
> > +  EXPECT_TRUE(allStylesEqual(LLVMStyles));
> > +
> > +  FormatStyle GoogleStyles[] = { getGoogleStyle(),
> getPredefinedStyle("Google"),
> > +                                 getPredefinedStyle("google"),
> > +                                 getPredefinedStyle("gOOgle") };
> > +  EXPECT_TRUE(allStylesEqual(GoogleStyles));
> > +
> > +  FormatStyle ChromiumStyles[] = { getChromiumStyle(),
> > +                                   getPredefinedStyle("Chromium"),
> > +                                   getPredefinedStyle("chromium"),
> > +                                   getPredefinedStyle("chROmiUM") };
> > +  EXPECT_TRUE(allStylesEqual(ChromiumStyles));
> > +
> > +  FormatStyle MozillaStyles[] = { getMozillaStyle(),
> > +                                  getPredefinedStyle("Mozilla"),
> > +                                  getPredefinedStyle("mozilla"),
> > +                                  getPredefinedStyle("moZilla") };
> > +  EXPECT_TRUE(allStylesEqual(MozillaStyles));
> > +}
> > +
> > +TEST_F(FormatTest, ParsesConfiguration) {
> > +  FormatStyle Style = {};
> > +#define CHECK_PARSE(TEXT, FIELD, VALUE)
>        \
> > +  EXPECT_NE(VALUE, Style.FIELD);
>         \
> > +  EXPECT_EQ(0, parseConfiguration(TEXT, &Style).value());
>        \
> > +  EXPECT_EQ(VALUE, Style.FIELD)
> > +
> > +#define CHECK_PARSE_BOOL(FIELD)
>        \
> > +  Style.FIELD = false;
>         \
> > +  EXPECT_EQ(0, parseConfiguration(#FIELD ": true", &Style).value());
>         \
> > +  EXPECT_EQ(true, Style.FIELD);
>        \
> > +  EXPECT_EQ(0, parseConfiguration(#FIELD ": false", &Style).value());
>        \
> > +  EXPECT_EQ(false, Style.FIELD);
> > +
> > +  CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
> > +  CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
> > +  CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
> > +  CHECK_PARSE_BOOL(BinPackParameters);
> > +  CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
> > +  CHECK_PARSE_BOOL(DerivePointerBinding);
> > +  CHECK_PARSE_BOOL(IndentCaseLabels);
> > +  CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
> > +  CHECK_PARSE_BOOL(PointerBindsToType);
> > +
> > +  CHECK_PARSE("AccessModifierOffset: -1234", AccessModifierOffset,
> -1234);
> > +  CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);
> > +  CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);
> > +  CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter,
> 1234u);
> > +  CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
> > +              PenaltyReturnTypeOnItsOwnLine, 1234u);
> > +  CHECK_PARSE("SpacesBeforeTrailingComments: 1234",
> > +              SpacesBeforeTrailingComments, 1234u);
> > +
> > +  Style.Standard = FormatStyle::LS_Auto;
> > +  CHECK_PARSE("Standard: C++03", Standard, FormatStyle::LS_Cpp03);
> > +  CHECK_PARSE("Standard: C++11", Standard, FormatStyle::LS_Cpp11);
> > +  CHECK_PARSE("Standard: Auto", Standard, FormatStyle::LS_Auto);
> > +
> > +  Style.ColumnLimit = 123;
> > +  FormatStyle BaseStyle = getLLVMStyle();
> > +  CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
> > +  CHECK_PARSE("BasedOnStyle: LLVM\nColumnLimit: 1234", ColumnLimit,
> 1234u);
> > +
> > +#undef CHECK_PARSE
> > +#undef CHECK_PARSE_BOOL
> > +}
> > +
> > +TEST_F(FormatTest, ConfigurationRoundTripTest) {
> > +  FormatStyle Style = getLLVMStyle();
> > +  std::string YAML = configurationAsText(Style);
> > +  FormatStyle ParsedStyle = {};
> > +  EXPECT_EQ(0, parseConfiguration(YAML, &ParsedStyle).value());
>
> This unit test is failing because of Input::beginMapping in
> YAMLTraits.cpp.  It uses dyn_cast, but CurrentNode is null (and so it
> asserts). I don't know enough about YAML to know whether CurrentNode
> being null is sensible or not.


The failure may be related to this bug in YAML I/O:
http://llvm.org/bugs/show_bug.cgi?id=15927, but this test passes for me
(linux on amd64, build type - none, with assertions). Could you provide
more details on your build configuration/environment? Inserting llvm::errs()
<< "<" << YAML << ">\n"; after configurationAsText(Style) call could also
be helpful.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130510/2e3d7903/attachment.html>


More information about the cfe-commits mailing list