r181326 - Config file support for clang-format, part 1.
Aaron Ballman
aaron at aaronballman.com
Fri May 10 07:31:45 PDT 2013
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.
More information about the cfe-commits
mailing list