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

Aaron Ballman aaron at aaronballman.com
Fri May 10 08:08:04 PDT 2013


Filed as http://llvm.org/bugs/show_bug.cgi?id=15960, and I'll disable
the test for now on Windows.

Thanks!

~Aaron

On Fri, May 10, 2013 at 10:51 AM, Alexander Kornienko <alexfh at google.com> wrote:
> Unfortunately, I don't have a Windows machine to test this, but I believe
> the bug is inside YAML I/O itself. If you can provide more information, I'd
> suggest that you file a bug with CC to Nick Kledzik <kledzik at apple.com>. And
> meanwhile you can disable the test on Windows with #ifndef _WIN32.
>
>
> On Fri, May 10, 2013 at 4:31 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> After fetching today (I saw you did part 2), I get different failures.
>>
>> 182>  FAIL: Clang-Unit ::
>>
>> Format/F:\llvm\build\tools\clang\unittests\Format\Debug/FormatTests.exe/FormatTest.ConfigurationRoundTripTest
>> (6013 of 6255)
>> 182>  ******************** TEST 'Clang-Unit ::
>>
>> Format/F:\llvm\build\tools\clang\unittests\Format\Debug/FormatTests.exe/FormatTest.ConfigurationRoundTripTest'
>> FAILED ********************
>> 182>  Note: Google Test filter = FormatTest.ConfigurationRoundTripTest
>> 182>
>> 182>  [==========] Running 1 test from 1 test case.
>> 182>
>> 182>  [----------] Global test environment set-up.
>> 182>
>> 182>  [----------] 1 test from FormatTest
>> 182>
>> 182>  [ RUN      ] FormatTest.ConfigurationRoundTripTest
>> 182>
>> 182>F:\llvm\llvm\tools\clang\unittests\Format\FormatTest.cpp(4038):
>> error : Value of: parseConfiguration(YAML, &ParsedStyle).value()
>> 182>
>> 182>    Actual: 22
>> 182>
>> 182>  Expected: 0
>> 182>
>> 182>  [  FAILED  ] FormatTest.ConfigurationRoundTripTest (2 ms)
>> 182>
>> 182>  [----------] 1 test from FormatTest (2 ms total)
>> 182>
>> 182>
>> 182>
>> 182>  [----------] Global test environment tear-down
>> 182>
>> 182>  [==========] 1 test from 1 test case ran. (2 ms total)
>> 182>
>> 182>  [  PASSED  ] 0 tests.
>> 182>
>> 182>  [  FAILED  ] 1 test, listed below:
>> 182>
>> 182>  [  FAILED  ] FormatTest.ConfigurationRoundTripTest
>> 182>
>> 182>
>> 182>
>> 182>   1 FAILED TEST
>> 182>
>> 182>  YAML:18:8: error: unknown key 'Standar'
>> 182>  Standar
>> 182>         ^
>> 182>
>> 182>  ********************
>> 182>
>> 182>  Testing Time: 112.79s
>> 182>  ********************
>> 182>  Failing Tests (1):
>> 182>      Clang-Unit ::
>>
>> Format/F:\llvm\build\tools\clang\unittests\Format\Debug/FormatTests.exe/FormatTest.ConfigurationRoundTripTest
>> 182>
>> 182>    Expected Passes    : 6148
>> 182>    Expected Failures  : 27
>> 182>    Unsupported Tests  : 79
>> 182>    Unexpected Failures: 1
>>
>> So the original failure seems to be gone, but a new one has popped up
>> in its place.  This time I am testing with MSVC 11 on Windows 7 x64 as
>> a debug build.
>>
>> ~Aaron
>>
>> On Thu, May 9, 2013 at 9:38 PM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>> >
>> > 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.
>
>
>
>
> --
> Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151 221 77
> 957
> Google Germany GmbH | Dienerstr. 12 | 80331 M√ľnchen




More information about the cfe-commits mailing list